Support input method and text input#1203
Conversation
|
Script for easier testing: |
|
Thanks. I intend to get around to testing this over the weekend. |
types/wlr_input_method.c
Outdated
| context->pending.preedit.cursor_end = cursor_end; | ||
| free(context->pending.preedit.text); | ||
| context->pending.preedit.text = strdup(text); // TODO: clean up | ||
| } |
types/wlr_input_method.c
Outdated
| struct wlr_input_method *context = | ||
| wl_resource_get_user_data(resource); | ||
| free(context->pending.commit_text); | ||
| context->pending.commit_text = strdup(text); // TODO: clean up |
| 'text-input': { | ||
| 'src': 'text-input.c', | ||
| 'dep': [wayland_cursor, wayland_client, wlr_protos, wlroots], | ||
| }, |
There was a problem hiding this comment.
You're adding both in the same commi alhough one should be in text-input and the other one in input-method.
There was a problem hiding this comment.
Should be fixed now.
| 'input-method-unstable-v2.xml', | ||
| 'screenshooter.xml', | ||
| 'text-input-unstable-v3.xml', | ||
| 'wlr-export-dmabuf-unstable-v1.xml', |
There was a problem hiding this comment.
You're adding both in the same commit although one should be in text input and the other one in input-method (to make reverts, etc simpler).
There was a problem hiding this comment.
Should be fixed now.
ddevault
left a comment
There was a problem hiding this comment.
A lot of this still looks WIP, definitely fix up the TODOs and FIXMEs. Overall code looks good
examples/input-method.c
Outdated
| @@ -0,0 +1,214 @@ | |||
| #ifndef _POSIX_C_SOURCE | |||
| #define _POSIX_C_SOURCE 200809L | |||
| #endif | |||
There was a problem hiding this comment.
No need to wrap this in ifndef
| } | ||
|
|
||
| return EXIT_SUCCESS; | ||
| } |
There was a problem hiding this comment.
This whole file looks pretty wip
examples/text-input.c
Outdated
| eglSwapBuffers(egl.display, egl_surface); | ||
| } | ||
|
|
||
| static void noop() {} |
There was a problem hiding this comment.
/* This space intentionally left blank */
examples/text-input.c
Outdated
| void text_input_handle_leave(void *data, | ||
| struct zwp_text_input_v3 *zwp_text_input_v3, | ||
| struct wl_surface *surface) { | ||
| enabled = 2; |
There was a problem hiding this comment.
This variable is poorly named
examples/text-input.c
Outdated
| } else if (strcmp(interface, zwp_text_input_manager_v3_interface.name) == 0) { | ||
| text_input_manager = wl_registry_bind(registry, name, | ||
| &zwp_text_input_manager_v3_interface, 1); | ||
|
|
include/wlr/types/wlr_input_method.h
Outdated
| #include <stdlib.h> | ||
| #include <wayland-server.h> | ||
|
|
||
|
|
There was a problem hiding this comment.
Extra newline here and on line 3
include/wlr/types/wlr_input_method.h
Outdated
| struct wl_global *input_method_manager; | ||
| struct wl_resource *resource; // FIXME: turn into a list, each entry per-client | ||
|
|
||
| struct wlr_input_method *input_method; // FIXME: turn into a list, each entry per-seat |
There was a problem hiding this comment.
The first FIXME pertains to a list of all input methods - since input methods are meant to take over inputs using grabs, does it make sense to allow multiple ones to be present at the same time?
(this doesn't actually change the shape of the struct, but still)
(after thinking a little, the answer is already in the code - each seat has a unique input method, and no one seems to have complained yet)
types/wlr_text_input.c
Outdated
| void *data) { | ||
| struct wlr_text_input *text_input = wl_container_of(listener, text_input, | ||
| seat_destroy_listener); | ||
| // wlr_text_input_destroy(text_input); TODO: what to do when the seat goes under? |
types/wlr_text_input.c
Outdated
| static void text_input_enable(struct wl_client *client, | ||
| struct wl_resource *resource) { | ||
| struct wlr_text_input *text_input = text_input_from_resource(resource); | ||
| struct wlr_text_input_properties defaults = {0}; |
types/wlr_input_method.c
Outdated
| .preedit_string = preedit_string, | ||
| .delete_surrounding_text = delete_surrounding_text, | ||
| .get_input_popup_surface = noop, | ||
| .grab_keyboard = noop, |
There was a problem hiding this comment.
I assume the noops here are still on the TODO list, correct?
There was a problem hiding this comment.
Yes, I wanted to get the whole concept into shape before I tackle these. They can be placed on top of the current patch independently, which helps to avoid a huge change at once.
|
[2018-08-31 07:17] dcz-purism <notifications@github.com>
Script for easier testing:
[...]
I tested the basic virtual keyboard functionality using the instructions
you provided and can confirm that it works!
It would definitely be good to be able to do some testing with more
complex input methods, i. e. ones that show word suggestions
for example. Is the plan to add something like that to
wlroots/examples/input-method.c?
|
examples/input-method.c
Outdated
|
|
||
| static void handle_unavailable(void *data, | ||
| struct zwp_input_method_v2 *zwp_input_method_v2) { | ||
| printf("IM disappeared"); |
examples/text-input.c
Outdated
| #include "text-input-unstable-v3-client-protocol.h" | ||
| #include "xdg-shell-client-protocol.h" | ||
|
|
||
| #include <linux/input-event-codes.h> |
There was a problem hiding this comment.
Add in FreeBSD too. You can find examples of how to do this by grepping other examples.
examples/input-method.c
Outdated
| #include "input-method-unstable-v2-client-protocol.h" | ||
| #include "xdg-shell-client-protocol.h" | ||
|
|
||
| #include <linux/input-event-codes.h> |
include/wlr/types/wlr_input_method.h
Outdated
| @@ -0,0 +1,73 @@ | |||
| #ifndef WLR_TYPES_WLR_INPUT_METHOD_H | |||
There was a problem hiding this comment.
This file should have a _v2 suffix
include/wlr/types/wlr_text_input.h
Outdated
| @@ -0,0 +1,77 @@ | |||
| #ifndef WLR_TYPES_WLR_TEXT_INPUT_H | |||
There was a problem hiding this comment.
This file should have a _v3 suffix (alongside with all structs).
Ah, and you need to add the WLR_USE_UNSTABLE stuff too at the beginning of all new header files.
include/wlr/types/wlr_text_input.h
Outdated
| }; | ||
|
|
||
| struct wlr_text_input_manager { | ||
| struct wl_global *wl_global; |
include/wlr/types/wlr_text_input.h
Outdated
|
|
||
| struct { | ||
| struct wl_signal text_input; // (struct wlr_text_input*) | ||
| } events; |
There was a problem hiding this comment.
The destroy event is issued on the text input itself: https://github.com/swaywm/wlroots/pull/1203/files/865e8dbad0d1d8ce5d7063f86f8b82cab38e9ad5#diff-c1e940621b62148527bc6252bfaeb83aR48
There was a problem hiding this comment.
I mean a destroy event on the manager. The manager gets destroyed with the display. If the compositor has objects that depend on it it needs to destroy them with the manager (it can't destroy them with the display because the destroy listener of the manager may be triggered before the destroy listener of the objects, this has caused issues in the past).
include/wlr/types/wlr_text_input.h
Outdated
|
|
||
| struct wlr_text_input_manager { | ||
| struct wl_global *wl_global; | ||
| struct wl_list text_inputs; // wlr_text_input::resource::link |
There was a problem hiding this comment.
Hmm, you mean wlr_text_input::link?
include/wlr/types/wlr_text_input.h
Outdated
| #include <wlr/types/wlr_seat.h> | ||
| #include <wlr/types/wlr_surface.h> | ||
|
|
||
| struct wlr_text_input_properties { |
There was a problem hiding this comment.
We generally use state instead of properties
include/wlr/types/wlr_text_input.h
Outdated
| uint32_t anchor; | ||
| } surrounding; | ||
|
|
||
| uint32_t text_change_cause; |
There was a problem hiding this comment.
Can this be an enum? (applies to other fields too)
include/wlr/types/wlr_text_input.h
Outdated
| struct wlr_surface *focused_surface; | ||
| struct wlr_text_input_properties pending; | ||
| struct wlr_text_input_properties current; | ||
| uint32_t current_serial; // next to send |
There was a problem hiding this comment.
If it's the next serial, maybe we should rename to next_serial?
types/wlr_input_method.c
Outdated
| static void commit(struct wl_client *client, struct wl_resource *resource, | ||
| uint32_t serial) { | ||
| struct wlr_input_method *context = | ||
| wl_resource_get_user_data(resource); |
There was a problem hiding this comment.
Should add a safe wrapper input_method_from_resource for this
types/wlr_input_method.c
Outdated
| context->current_serial = serial; | ||
| struct wlr_input_method_state default_state = {0}; | ||
| context->pending = default_state; | ||
| wlr_signal_emit_safe(&context->events.commit, (void*)context); |
types/wlr_input_method.c
Outdated
|
|
||
| static void input_method_handle_resource_destroy(struct wl_resource *resource) { | ||
| struct wlr_input_method *context = input_method_from_resource(resource); | ||
| if (!context) { |
There was a problem hiding this comment.
input_method_destroy should check for NULL
There was a problem hiding this comment.
Does it apply to all destructors? The check was something I realized is not needed.
There was a problem hiding this comment.
No, but that's how inert resources work. See the last part of CONTRIBUTING.md.
There was a problem hiding this comment.
Thanks, I didn't realize this guide was there.
Yes, I want to create additional examples for testing once I start working on the floating popup and keyboard grab interfaces. |
types/wlr_input_method.c
Outdated
|
|
||
| static void manager_destroy(struct wl_client *client, | ||
| struct wl_resource *resource) { | ||
| // FIXME |
There was a problem hiding this comment.
This should call wl_resource_destroy
types/wlr_text_input.c
Outdated
| wl_list_remove(&text_input->link); | ||
| // no need to destroy the resource if client closed, prompting the server to do it | ||
| if (text_input->resource) { | ||
| wl_resource_destroy(text_input->resource); |
There was a problem hiding this comment.
We should never do this. Instead, the client should always send a request to destroy the resource.
types/wlr_text_input.c
Outdated
|
|
||
| static void text_input_destroy(struct wl_client *client, | ||
| struct wl_resource *resource) { | ||
| text_input_resource_destroy(resource); |
There was a problem hiding this comment.
This should just call wl_resource_destroy
|
bump |
wayland-scanner >= 1.15.0 accepts foreign struct references, necessary in protocols like zwp-input-method-v2
Implemented basic input method functionality. Not included: popups, grabbing.
The compositor acts as a relay between applications using the text-input protocol and input methods using the input-method protocol. This change implements the basic but useful support for input-method, leaving out grabs as well as popups.
|
Bump |
|
Well, I like it. Thanks! |
|
It seems this merge has broken the build on an |
|
Send a patch! |
|
Thank you! |
|
@Edootjuh I can't reproduce this, how are you compiling it? |
|
@SirCmpwn thanks for fixing this up so quickly. |
|
@dcz-purism I pushed a fix |
This ports swaywm/wlroots#1203, swaywm/wlroots#1303, swaywm/wlroots#1308, swaywm/wlroots#1759 rootston part to sway.
This ports swaywm/wlroots#1203, swaywm/wlroots#1303, swaywm/wlroots#1308, swaywm/wlroots#1759 rootston part to sway.
This ports swaywm/wlroots#1203, swaywm/wlroots#1303, swaywm/wlroots#1308, swaywm/wlroots#1759 rootston part to sway. Co-Authored-By: Leo Chen <leo881003@gmail.com>
This ports swaywm/wlroots#1203, swaywm/wlroots#1303, swaywm/wlroots#1308, swaywm/wlroots#1759 rootston part to sway. Co-Authored-By: Leo Chen <leo881003@gmail.com>
This ports swaywm/wlroots#1203, swaywm/wlroots#1303, swaywm/wlroots#1308, swaywm/wlroots#1759 rootston part to sway. Co-Authored-By: Leo Chen <leo881003@gmail.com>
This ports swaywm/wlroots#1203, swaywm/wlroots#1303, swaywm/wlroots#1308, swaywm/wlroots#1759 rootston part to sway. Co-Authored-By: Leo Chen <leo881003@gmail.com>
This ports swaywm/wlroots#1203, swaywm/wlroots#1303, swaywm/wlroots#1308, swaywm/wlroots#1759 rootston part to sway. Co-Authored-By: Leo Chen <leo881003@gmail.com>
This ports swaywm/wlroots#1203, swaywm/wlroots#1303, swaywm/wlroots#1308, swaywm/wlroots#1759 rootston part to sway. Co-Authored-By: Leo Chen <leo881003@gmail.com>
This ports swaywm/wlroots#1203, swaywm/wlroots#1303, swaywm/wlroots#1308, swaywm/wlroots#1759 rootston part to sway. Co-Authored-By: Leo Chen <leo881003@gmail.com>
This ports swaywm/wlroots#1203, swaywm/wlroots#1303, swaywm/wlroots#1308, swaywm/wlroots#1759 rootston part to sway. Co-Authored-By: Leo Chen <leo881003@gmail.com>
This ports swaywm/wlroots#1203, swaywm/wlroots#1303, swaywm/wlroots#1308, swaywm/wlroots#1759 rootston part to sway. Co-Authored-By: Leo Chen <leo881003@gmail.com>
This ports swaywm/wlroots#1203, swaywm/wlroots#1303, swaywm/wlroots#1308, swaywm/wlroots#1759 rootston part to sway. Co-Authored-By: Leo Chen <leo881003@gmail.com>
This ports swaywm/wlroots#1203, swaywm/wlroots#1303, swaywm/wlroots#1308, swaywm/wlroots#1759 rootston part to sway. Co-Authored-By: Leo Chen <leo881003@gmail.com>
This ports swaywm/wlroots#1203, swaywm/wlroots#1303, swaywm/wlroots#1308, swaywm/wlroots#1759 rootston part to sway. Co-Authored-By: Leo Chen <leo881003@gmail.com>
This change introduces basic but useful support for
zwp-text-input-v3andzwp-input-method-v2protocols.The compositor becomes a relay between the input method program and the user-facing text input application, passing messages between them, and gracefully handling connections and disconnections.
The text-input protocol is implemented fully. The basic and least controversial pieces to make input-method functionality complete are implemented as well. Remaining are the grab and popup functionality. What's currently implemented is enough to support virtual keyboards, but probably not enough for e.g. CJK input methods.
The structure doing most of the work is the
rootston/text_input.h::roots_input_method_relay. One is attached to each seat, and each manages multiple text-input connections and zero or one input-method connections.Testing possible using virtboard's input_method branch (at scaling factor x1) and gtk-3.24.
To test with system-wide GTK:
With locally-installed GTK, the
yadline can look like this:or, in the unlikely case of issues: