Adding text_input_v3 and text_input_v2 in order to add IME support - #766
Adding text_input_v3 and text_input_v2 in order to add IME support#766myf wants to merge 1 commit into
Conversation
JimmyCozza
left a comment
JimmyCozza
left a comment
There was a problem hiding this comment.
Thanks for the PR. There are some things I'm trying to make sure I understand and think about.
Could you also add some tests? Nothing in the suite speaks text-input-v3 or input-method-v2 yet, so this needs a small test client that can act as both a text input and a fake input method.
| } | ||
|
|
||
| static void | ||
| handle_focused_surface_unmap(struct wl_listener *listener, void *data) |
There was a problem hiding this comment.
The popup tree is a child of c->popups, which unmapnotify already destroys. Does this run after that and destroy it a second time?
| * from the input method instead; the input method itself may already be | ||
| * gone if the grab is being torn down as part of its destruction. */ | ||
| struct wlr_input_method_keyboard_grab_v2 *keyboard_grab = | ||
| im_relay.input_method ? im_relay.input_method->keyboard_grab : NULL; |
There was a problem hiding this comment.
On wlroots 0.19 the IM destroy fires before the grab destroy, so this is NULL right when fcitx5 exits. Should the grab be stored at grab time instead?
| struct wlr_input_method_keyboard_grab_v2 *keyboard_grab = data; | ||
|
|
||
| wlr_input_method_keyboard_grab_v2_set_keyboard(keyboard_grab, | ||
| wlr_seat_get_keyboard(seat)); |
There was a problem hiding this comment.
When fcitx5's virtual keyboard was the last one used, isn't this the IME's own keyboard? Why not the physical group?
| } | ||
|
|
||
| static void | ||
| relay_handle_new_text_input(struct wl_listener *listener, void *data) |
There was a problem hiding this comment.
If the surface already has keyboard focus when this is created, what sends enter? Sway checks the focused surface here.
| popup_height = popup->popup_surface->surface->current.height; | ||
|
|
||
| /* Layout-space edges of the cursor rectangle */ | ||
| x1 = parent_lx + cursor_area.x; |
There was a problem hiding this comment.
For XDG clients c->popups sits at the window-geometry origin, but cursor_rectangle is surface-local. Is this off by geometry.x/y for CSD clients?
| x = x1; | ||
| y = y2; | ||
|
|
||
| output = wlr_output_layout_output_at(output_layout, x1, y1); |
There was a problem hiding this comment.
popup_unconstrain in window.c uses the monitor workarea. Should this reuse it so the candidate window avoids the wibar?
|
|
||
| /* An input method holding the keyboard grab consumes what the compositor | ||
| * did not. Checked after keybindings so bindings still win over the IME. */ | ||
| if ((kb_grab = input_method_get_keyboard_grab(group))) { |
There was a problem hiding this comment.
handled only latches on presses. Does a release for a consumed keybinding still reach the grab, and then come back through the vk?
|
|
||
| /* An input method holding the keyboard grab consumes what the compositor | ||
| * did not. Checked after keybindings so bindings still win over the IME. */ | ||
| if ((kb_grab = input_method_get_keyboard_grab(group))) { |
There was a problem hiding this comment.
also
Should this be gated on locked like the keybinding branch above?
|
|
||
| /* Modifiers follow the keys: while an input method holds the grab they go | ||
| * to the IME rather than the focused client. */ | ||
| if ((kb_grab = input_method_get_keyboard_grab(group))) { |
There was a problem hiding this comment.
Same lock question as line 1229.
to Chinese input methods (fctix5, ibus). `zwp_text_input_v3` for clients and `zwp_input_method_v2` are implementd. ref: trip-zip#765 Changes - input_method.c / input_method.h — the relay. Adapted from sway's sway/input/text_input.c (MIT), with three departures for somewm: a single global relay rather than per-seat; focus tracked via seat->keyboard_state.events.focus_change instead of patching the nine wlr_seat_keyboard_notify_enter call sites; and IME popups parented onto the existing client_t::popups / LayerSurface::popups trees, which avoids sway's scene_descriptor machinery. - input.c — routes keys and modifiers to the input method's keyboard grab in keypress() / keypressmod(), after keybinding handling so compositor bindings still win. Records the originating virtual keyboard on KeyboardGroup so an IME's own injected keys aren't fed back to it. - wlr_compat.h — 0.20 renamed the manager new_text_input / new_input_method signals; macros keep 0.19 building. - somewm.c, somewm_types.h, meson.build — manager creation, relay init/finish, struct field, source entry. AI disclosure: written with Claude Code. The relay module was AI-drafted against sway as a reference and reviewed/modified by me; a NULL-deref crash in the keyboard-grab destroy handler was diagnosed from coredumps and fixed.
8549814 to
cb27602
Compare
2 participants
to Chinese input methods (fctix5, ibus).
zwp_text_input_v3for clients andzwp_input_method_v2are implementd.ref: #765
Changes
- input_method.c / input_method.h — the relay. Adapted from sway's sway/input/text_input.c (MIT), with three departures for somewm: a single global relay rather than per-seat; focus tracked via seat->keyboard_state.events.focus_change instead of patching the nine wlr_seat_keyboard_notify_enter call sites; and IME popups parented onto the existing client_t::popups / LayerSurface::popups trees, which avoids sway's scene_descriptor machinery.
- input.c — routes keys and modifiers to the input method's keyboard grab in keypress() / keypressmod(), after keybinding handling so compositor bindings still win. Records the originating virtual keyboard on KeyboardGroup so an IME's own injected keys aren't fed back to it.
- wlr_compat.h — 0.20 renamed the manager new_text_input / new_input_method signals; macros keep 0.19 building.
- somewm.c, somewm_types.h, meson.build — manager creation, relay init/finish, struct field, source entry.
AI disclosure: written with Claude Code. The relay module was AI-drafted against sway as a reference and reviewed/modified by me; a NULL-deref crash in the keyboard-grab destroy handler was diagnosed from coredumps and fixed.
Description
Test Plan
AI Usage
Checklist
lua/awful/,lua/gears/,lua/wibox/,lua/naughty/) are not modified — if a bug surfaces in Lua, the fix belongs in Cmake test-unit && make test-integration)