[PATCH 4/6] text: start support for keyboard events
Kristian Høgsberg
hoegsberg at gmail.com
Sun Jul 22 12:08:07 PDT 2012
On Sun, Jul 22, 2012 at 07:58:34PM +0200, Philipp Brüschweiler wrote:
> Hi Kristian,
>
> Thank you for including my patches. I'll try to follow the style
> guidelines from now on :)
Heh, sounds good. It was mostly all good though, just a few nit-picks.
> On Sun, Jul 22, 2012 at 6:24 PM, Kristian Høgsberg <hoegsberg at gmail.com> wrote:
> > On Wed, Jul 11, 2012 at 10:25:32PM +0200, Philipp Brüschweiler wrote:
> >> * add the request_keyboard_input request to signal that the
> >> input method wants to receive keyboard events
> >> * add key and modifier events
> >> * keyboard events are grabbed with wl_keyboard_grab and relayed
> >> to the input method
> >
> > Can we do this with the wl_keyboard.focus_signal instead?
>
> Do you mean installing a handler on the focus_signal instead of the
> custom keyboard_set_focus function? I'm pretty sure we can, I'll look
> into this.
Yes, instead of wrapping wl_keyboard_set_focus, it looks like you can
just add a handler. Should also make the patch a great deal smaller.
> > Also, this isn't lisp, we don't use _p for bools :) I don't think we
> > need the request_p argument in request_keyboard_input at all. If the
> > input_method doesn't need keyboard input, it just shouldn't send that
> > request. Also, I don't like how we're starting to duplicate the
> > wl_keyboard interface. Maybe we can combine the two and do something like
> >
> > <request name="request_keyboard">
> > <arg name="keyboard" type="new_id" interface="wl_keyboard"/>
> > </request>
> >
> > That is, reuse the wl_keyboard interface for keyboard input and use
> > this request to indicate that the input method wants keyboard input?
>
> This is a great idea. I'll try it and rework the remaining patches.
>
> Thanks for the review,
> Philipp
>
> > Kristian
> >
> >> ---
> >> protocol/text.xml | 28 +++++++++++
> >> src/compositor.c | 15 +++---
> >> src/compositor.h | 8 +++-
> >> src/shell.c | 10 ++--
> >> src/text-backend.c | 136 +++++++++++++++++++++++++++++++++++++++++++++++++++--
> >> 5 Dateien geändert, 180 Zeilen hinzugefügt(+), 17 Zeilen entfernt(-)
> >>
> >> diff --git a/protocol/text.xml b/protocol/text.xml
> >> index a14277a..e51057a 100644
> >> --- a/protocol/text.xml
> >> +++ b/protocol/text.xml
> >> @@ -48,5 +48,33 @@
> >> <arg name="text" type="string"/>
> >> <arg name="index" type="uint"/>
> >> </request>
> >> +
> >> + <request name="request_keyboard_input">
> >> + <arg name="request_p" type="uint" description="0: false, 1: true"/>
> >> + </request>
> >> +
> >> + <event name="key">
> >> + <description summary="key event">
> >> + A key was pressed or released.
> >> + </description>
> >> +
> >> + <arg name="serial" type="uint"/>
> >> + <arg name="time" type="uint"/>
> >> + <arg name="key" type="uint"/>
> >> + <arg name="state" type="uint"/>
> >> + </event>
> >> +
> >> + <event name="modifiers">
> >> + <description summary="modifier and group state">
> >> + Notifies clients that the modifier and/or group state has
> >> + changed, and it should update its local state.
> >> + </description>
> >> +
> >> + <arg name="serial" type="uint"/>
> >> + <arg name="mods_depressed" type="uint"/>
> >> + <arg name="mods_latched" type="uint"/>
> >> + <arg name="mods_locked" type="uint"/>
> >> + <arg name="group" type="uint"/>
> >> + </event>
> >> </interface>
> >> </protocol>
> >> diff --git a/src/compositor.c b/src/compositor.c
> >> index 678e0bd..e28ee4b 100644
> >> --- a/src/compositor.c
> >> +++ b/src/compositor.c
> >> @@ -646,7 +646,7 @@ weston_surface_unmap(struct weston_surface *surface)
> >> wl_list_for_each(seat, &surface->compositor->seat_list, link) {
> >> if (seat->seat.keyboard &&
> >> seat->seat.keyboard->focus == &surface->surface)
> >> - wl_keyboard_set_focus(seat->seat.keyboard, NULL);
> >> + keyboard_set_focus(seat->compositor, seat->seat.keyboard, NULL);
> >> if (seat->seat.pointer &&
> >> seat->seat.pointer->focus == &surface->surface)
> >> wl_pointer_set_focus(seat->seat.pointer,
> >> @@ -1686,7 +1686,7 @@ weston_surface_activate(struct weston_surface *surface,
> >> struct weston_compositor *compositor = seat->compositor;
> >>
> >> if (seat->seat.keyboard) {
> >> - wl_keyboard_set_focus(seat->seat.keyboard, &surface->surface);
> >> + keyboard_set_focus(seat->compositor, seat->seat.keyboard, &surface->surface);
> >> wl_data_device_set_keyboard_focus(&seat->seat);
> >> }
> >>
> >> @@ -1959,7 +1959,7 @@ notify_keyboard_focus_in(struct wl_seat *seat, struct wl_array *keys,
> >>
> >> if (surface) {
> >> wl_list_remove(&ws->saved_kbd_focus_listener.link);
> >> - wl_keyboard_set_focus(ws->seat.keyboard, surface);
> >> + keyboard_set_focus(ws->compositor, ws->seat.keyboard, surface);
> >> ws->saved_kbd_focus = NULL;
> >> }
> >> }
> >> @@ -1991,7 +1991,7 @@ notify_keyboard_focus_out(struct wl_seat *seat)
> >> &ws->saved_kbd_focus_listener);
> >> }
> >>
> >> - wl_keyboard_set_focus(ws->seat.keyboard, NULL);
> >> + keyboard_set_focus(ws->compositor, ws->seat.keyboard, NULL);
> >> /* FIXME: We really need keyboard grab cancel here to
> >> * let the grab shut down properly. As it is we leak
> >> * the grab data. */
> >> @@ -2293,8 +2293,9 @@ seat_get_keyboard(struct wl_client *client, struct wl_resource *resource,
> >>
> >> if (seat->seat.keyboard->focus &&
> >> seat->seat.keyboard->focus->resource.client == client) {
> >> - wl_keyboard_set_focus(seat->seat.keyboard,
> >> - seat->seat.keyboard->focus);
> >> + keyboard_set_focus(seat->compositor,
> >> + seat->seat.keyboard,
> >> + seat->seat.keyboard->focus);
> >> }
> >> }
> >>
> >> @@ -3044,7 +3045,7 @@ weston_compositor_init(struct weston_compositor *ec,
> >>
> >> screenshooter_create(ec);
> >> text_cursor_position_notifier_create(ec);
> >> - input_method_create(ec);
> >> + ec->input_method = input_method_create(ec);
> >>
> >> wl_data_device_manager_init(ec->wl_display);
> >>
> >> diff --git a/src/compositor.h b/src/compositor.h
> >> index 0d49dec..34c4778 100644
> >> --- a/src/compositor.h
> >> +++ b/src/compositor.h
> >> @@ -289,6 +289,8 @@ struct weston_compositor {
> >> /* There can be more than one, but not right now... */
> >> struct weston_seat *seat;
> >>
> >> + struct input_method *input_method;
> >> +
> >> struct weston_layer fade_layer;
> >> struct weston_layer cursor_layer;
> >>
> >> @@ -708,7 +710,7 @@ clipboard_create(struct weston_seat *seat);
> >> void
> >> text_cursor_position_notifier_create(struct weston_compositor *ec);
> >>
> >> -void
> >> +struct input_method *
> >> input_method_create(struct weston_compositor *ec);
> >>
> >> struct weston_process;
> >> @@ -761,4 +763,8 @@ backend_init(struct wl_display *display, int argc, char *argv[],
> >> int
> >> weston_output_switch_mode(struct weston_output *output, struct weston_mode *mode);
> >>
> >> +void
> >> +keyboard_set_focus(struct weston_compositor *compositor, struct wl_keyboard *keyboard,
> >> + struct wl_surface *surface);
> >> +
> >> #endif
> >> diff --git a/src/shell.c b/src/shell.c
> >> index 66c9d64..6504631 100644
> >> --- a/src/shell.c
> >> +++ b/src/shell.c
> >> @@ -421,8 +421,9 @@ pop_focus_state(struct desktop_shell *shell, struct workspace *ws)
> >>
> >> wl_list_for_each_safe(state, next, &ws->focus_list, link) {
> >> if (state->keyboard_focus)
> >> - wl_keyboard_set_focus(state->seat->seat.keyboard,
> >> - &state->keyboard_focus->surface);
> >> + keyboard_set_focus(shell->compositor,
> >> + state->seat->seat.keyboard,
> >> + &state->keyboard_focus->surface);
> >>
> >> focus_state_destroy(state);
> >> }
> >> @@ -445,7 +446,8 @@ push_focus_state(struct desktop_shell *shell, struct workspace *ws)
> >>
> >> wl_list_insert(&ws->focus_list, &state->link);
> >>
> >> - wl_keyboard_set_focus(seat->seat.keyboard, NULL);
> >> + keyboard_set_focus(shell->compositor,
> >> + seat->seat.keyboard, NULL);
> >> }
> >> }
> >> }
> >> @@ -3046,7 +3048,7 @@ switcher_binding(struct wl_seat *seat, uint32_t time, uint32_t key,
> >> lower_fullscreen_layer(switcher->shell);
> >> switcher->grab.interface = &switcher_grab;
> >> wl_keyboard_start_grab(seat->keyboard, &switcher->grab);
> >> - wl_keyboard_set_focus(seat->keyboard, NULL);
> >> + keyboard_set_focus(shell->compositor, seat->keyboard, NULL);
> >> switcher_next(switcher);
> >> }
> >>
> >> diff --git a/src/text-backend.c b/src/text-backend.c
> >> index e4dea8f..8681a95 100644
> >> --- a/src/text-backend.c
> >> +++ b/src/text-backend.c
> >> @@ -20,10 +20,13 @@
> >> * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> >> */
> >>
> >> +#include <stdbool.h>
> >> #include <stdlib.h>
> >> +#include <string.h>
> >>
> >> #include "compositor.h"
> >> #include "text-server-protocol.h"
> >> +#include "log.h"
> >>
> >> struct input_method;
> >>
> >> @@ -33,6 +36,8 @@ struct text_model {
> >> struct wl_list link;
> >>
> >> struct input_method *input_method;
> >> + struct wl_keyboard_grab grab;
> >> + struct wl_surface *surface;
> >> };
> >>
> >> struct input_method {
> >> @@ -44,15 +49,23 @@ struct input_method {
> >> struct weston_compositor *ec;
> >> struct wl_list models;
> >> struct text_model *active_model;
> >> +
> >> + bool request_keyboard_input_p;
> >> };
> >>
> >> static void
> >> deactivate_text_model(struct text_model *text_model)
> >> {
> >> + struct wl_keyboard_grab *grab = &text_model->grab;
> >> + if (grab->keyboard && (grab->keyboard->grab == grab)) {
> >> + wl_keyboard_end_grab(grab->keyboard);
> >> + weston_log("end keyboard grab\n");
> >> + }
> >>
> >> if (text_model->input_method->active_model == text_model) {
> >> text_model->input_method->active_model = NULL;
> >> - wl_signal_emit(&text_model->input_method->ec->hide_input_panel_signal, text_model->input_method->ec);
> >> + wl_signal_emit(&text_model->input_method->ec->hide_input_panel_signal,
> >> + text_model->input_method->ec);
> >> }
> >> }
> >>
> >> @@ -83,6 +96,40 @@ text_model_set_cursor_index(struct wl_client *client,
> >> }
> >>
> >> static void
> >> +text_model_key(struct wl_keyboard_grab *grab,
> >> + uint32_t time, uint32_t key, uint32_t state_w)
> >> +{
> >> + struct text_model *text_model = container_of(grab, struct text_model, grab);
> >> +
> >> + uint32_t serial = wl_display_next_serial(text_model->input_method->ec->wl_display);
> >> + if (text_model->input_method->input_method_binding) {
> >> + input_method_send_key(text_model->input_method->input_method_binding,
> >> + serial, time, key, state_w);
> >> + }
> >> +}
> >> +
> >> +static void
> >> +text_model_modifier(struct wl_keyboard_grab *grab, uint32_t serial,
> >> + uint32_t mods_depressed, uint32_t mods_latched,
> >> + uint32_t mods_locked, uint32_t group)
> >> +{
> >> + struct text_model *text_model = container_of(grab, struct text_model, grab);
> >> +
> >> + uint32_t new_serial = wl_display_next_serial(text_model->input_method->ec->wl_display);
> >> + if (text_model->input_method->input_method_binding) {
> >> + input_method_send_modifiers(text_model->input_method->input_method_binding,
> >> + new_serial, mods_depressed, mods_latched,
> >> + mods_locked, group);
> >> + }
> >> +}
> >> +
> >> +static const struct wl_keyboard_grab_interface text_model_grab = {
> >> + text_model_key,
> >> + text_model_modifier,
> >> +};
> >> +
> >> +
> >> +static void
> >> text_model_activate(struct wl_client *client,
> >> struct wl_resource *resource)
> >> {
> >> @@ -90,7 +137,19 @@ text_model_activate(struct wl_client *client,
> >>
> >> text_model->input_method->active_model = text_model;
> >>
> >> - wl_signal_emit(&text_model->input_method->ec->show_input_panel_signal, text_model->input_method->ec);
> >> + wl_signal_emit(&text_model->input_method->ec->show_input_panel_signal,
> >> + text_model->input_method->ec);
> >> +
> >> + if (text_model->input_method->input_method_binding &&
> >> + text_model->input_method->request_keyboard_input_p) {
> >> + struct wl_seat *seat = &text_model->input_method->ec->seat->seat;
> >> +
> >> + if (seat->keyboard->grab != &seat->keyboard->default_grab) {
> >> + wl_keyboard_end_grab(seat->keyboard);
> >> + }
> >> + wl_keyboard_start_grab(seat->keyboard, &text_model->grab);
> >> + weston_log("start keyboard grab\n");
> >> + }
> >> }
> >>
> >> static void
> >> @@ -162,6 +221,9 @@ static void text_model_manager_create_text_model(struct wl_client *client,
> >> text_model->resource.data = text_model;
> >>
> >> text_model->input_method = input_method;
> >> + text_model->surface = container_of(surface, struct wl_surface, resource);
> >> +
> >> + text_model->grab.interface = &text_model_grab;
> >>
> >> wl_client_add_resource(client, &text_model->resource);
> >>
> >> @@ -196,21 +258,66 @@ input_method_commit_string(struct wl_client *client,
> >> struct input_method *input_method = resource->data;
> >>
> >> if (input_method->active_model) {
> >> - text_model_send_commit_string(&input_method->active_model->resource, text, index);
> >> + text_model_send_commit_string(&input_method->active_model->resource,
> >> + text, index);
> >> + }
> >> +}
> >> +
> >> +static void
> >> +input_method_request_keyboard_input(struct wl_client *client,
> >> + struct wl_resource *resource,
> >> + uint32_t request_p)
> >> +{
> >> +
> >> + struct input_method *input_method = resource->data;
> >> +
> >> + weston_log("request keyboard!\n");
> >> + if ((input_method->request_keyboard_input_p != (bool) request_p)
> >> + && input_method->active_model) {
> >> + if (request_p) {
> >> + struct wl_seat *seat = &input_method->ec->seat->seat;
> >> +
> >> + wl_keyboard_start_grab(seat->keyboard,
> >> + &input_method->active_model->grab);
> >> + } else {
> >> + struct wl_keyboard_grab *grab = &input_method->active_model->grab;
> >> + if (grab->keyboard && grab->keyboard->grab == grab) {
> >> + wl_keyboard_end_grab(grab->keyboard);
> >> + }
> >> + }
> >> }
> >> +
> >> + input_method->request_keyboard_input_p = request_p;
> >> }
> >>
> >> static const struct input_method_interface input_method_implementation = {
> >> - input_method_commit_string
> >> + input_method_commit_string,
> >> + input_method_request_keyboard_input
> >> };
> >>
> >> static void
> >> +clear_input_method_data(struct input_method *input_method)
> >> +{
> >> + input_method->request_keyboard_input_p = false;
> >> +}
> >> +
> >> +static void
> >> unbind_input_method(struct wl_resource *resource)
> >> {
> >> struct input_method *input_method = resource->data;
> >>
> >> + if (input_method->active_model) {
> >> + struct wl_keyboard_grab *grab = &input_method->active_model->grab;
> >> + if (grab->keyboard && (grab->keyboard->grab == grab)) {
> >> + wl_keyboard_end_grab(grab->keyboard);
> >> + weston_log("end keyboard grab\n");
> >> + }
> >> + }
> >> +
> >> input_method->input_method_binding = NULL;
> >> free(resource);
> >> +
> >> + clear_input_method_data(input_method);
> >> }
> >>
> >> static void
> >> @@ -249,15 +356,17 @@ input_method_notifier_destroy(struct wl_listener *listener, void *data)
> >> free(input_method);
> >> }
> >>
> >> -void
> >> +WL_EXPORT struct input_method *
> >> input_method_create(struct weston_compositor *ec)
> >> {
> >> +
> >> struct input_method *input_method;
> >>
> >> input_method = calloc(1, sizeof *input_method);
> >>
> >> input_method->ec = ec;
> >> input_method->active_model = NULL;
> >> + clear_input_method_data(input_method);
> >>
> >> wl_list_init(&input_method->models);
> >>
> >> @@ -273,4 +382,21 @@ input_method_create(struct weston_compositor *ec)
> >>
> >> input_method->destroy_listener.notify = input_method_notifier_destroy;
> >> wl_signal_add(&ec->destroy_signal, &input_method->destroy_listener);
> >> +
> >> + return input_method;
> >> +}
> >> +
> >> +WL_EXPORT void
> >> +keyboard_set_focus(struct weston_compositor *compositor,
> >> + struct wl_keyboard *keyboard,
> >> + struct wl_surface *surface)
> >> +{
> >> + if (compositor->input_method) {
> >> + struct text_model *active_model =
> >> + compositor->input_method->active_model;
> >> + if (active_model && active_model->surface != surface) {
> >> + deactivate_text_model(active_model);
> >> + }
> >> + }
> >> + wl_keyboard_set_focus(keyboard, surface);
> >> }
> >> --
> >> 1.7.11.1
> >>
> >> _______________________________________________
> >> wayland-devel mailing list
> >> wayland-devel at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list