[PATCH weston 2/3] input: Add support for making libxkbcommon optional

[Pekka Paalanen]

On Fri, 14 Jun 2013 17:42:08 +0100
Rob Bradford <robert.bradford at intel.com> wrote:

> From: Matt Roper <matthew.d.roper at intel.com>
> 
> In embedded environments, devices that appear as evdev "keyboards" often
> have no resemblence to PC-style keyboards.  It is not uncommon for such
> environments to have no concept of modifier keys and no need for XKB key
> mapping; in these cases libxkbcommon initialization becomes unnecessary
> startup overhead.  On some SOC platforms, xkb keymap compilation can
> account for as much as 1/3 - 1/2 of the total compositor startup time.
> 
> This patch introduces a 'use_xkbcommon' flag in the core compositor
> structure that indicates whether the compositor is running in "raw
> keyboard" mode.  In raw keyboard mode, the compositor bypasses all
> libxkbcommon initialization and processing.  'key' events containing the
> integer keycode will continue to be delivered via the wl_keyboard
> interface, but no 'keymap' event will be sent to clients.  No modifier
> handling or keysym mapping is performed in this mode.
> 
> Note that upstream sample apps (e.g., weston-terminal or the
> desktop-shell client) will not recognize raw keycodes and will not react
> to keypresses when the compositor is operating in raw keyboard mode.
> This is expected behavior; key events are still being sent to the
> client, the client (and/or its toolkit) just isn't written to handle
> keypresses without doing xkb keysym mapping.  Applications written
> specifically for such embedded environments would be handling keypresses
> via the raw keycode delivered as part of the 'key' event rather than
> using xkb keysym mapping.
> 
> Whether to use xkbcommon is a global option that applies to all
> compositor keyboard devices on the system; it is an all-or-nothing flag.
> This patch simply adds conditional checks on whether xkbcommon is to be
> used or not.
> 
> v2 by Rob Bradford <rob at linux.intel.com>: the original version of the
> patch used a "raw_keycodes" flag instead of the "use_xkbcommon" used in
> this patch.
> 
> v1: Reviewed-by: Singh, Satyeshwar <satyeshwar.singh at intel.com>
> v1: Reviewed-by: Bob Paauwe <bob.j.paauwe at intel.com>

Hi,

sending raw keycodes sounds crazy, given e.g. the problems we will
have with gamepads elsewhere, but I'll leave that for others to
criticise, since I'm not too familiar with keyboard input.

Did you investigate writing a small, custom xkb keymap for your
device?

> ---
>  src/compositor.c |  1 +
>  src/compositor.h |  3 +++
>  src/input.c      | 71 +++++++++++++++++++++++++++++++++++++++-----------------
>  3 files changed, 54 insertions(+), 21 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 42011f5..fc407dd 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -2811,6 +2811,7 @@ weston_compositor_init(struct weston_compositor *ec,
>  	weston_plane_init(&ec->primary_plane, 0, 0);
>  	weston_compositor_stack_plane(ec, &ec->primary_plane, NULL);
>  
> +	ec->use_xkbcommon = 1;
>  	s = weston_config_get_section(ec->config, "keyboard", NULL, NULL);
>  	weston_config_section_get_string(s, "keymap_rules",
>  					 (char **) &xkb_names.rules, NULL);
> diff --git a/src/compositor.h b/src/compositor.h
> index c87d9f1..b80388e 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -558,6 +558,9 @@ struct weston_compositor {
>  	struct xkb_rule_names xkb_names;
>  	struct xkb_context *xkb_context;
>  	struct weston_xkb_info xkb_info;
> +
> +	/* Raw keyboard processing (no libxkbcommon initialization or handling) */
> +	int use_xkbcommon;
>  };
>  
>  struct weston_buffer_reference {
> diff --git a/src/input.c b/src/input.c
> index 5463d78..8ffbd10 100644
> --- a/src/input.c
> +++ b/src/input.c
> @@ -815,6 +815,10 @@ update_modifier_state(struct weston_seat *seat, uint32_t serial, uint32_t key,
>  {
>  	enum xkb_key_direction direction;
>  
> +	/* Keyboard modifiers don't exist in raw keyboard mode */
> +	if (!seat->compositor->use_xkbcommon)
> +		return;
> +
>  	if (state == WL_KEYBOARD_KEY_STATE_PRESSED)
>  		direction = XKB_KEY_DOWN;
>  	else
> @@ -1199,9 +1203,15 @@ seat_get_keyboard(struct wl_client *client, struct wl_resource *resource,
>  	wl_list_insert(&seat->keyboard->resource_list, &cr->link);
>  	cr->destroy = unbind_resource;
>  
> -	wl_keyboard_send_keymap(cr, WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1,
> -				seat->xkb_info.keymap_fd,
> -				seat->xkb_info.keymap_size);
> +	if (seat->compositor->use_xkbcommon) {
> +		wl_keyboard_send_keymap(cr, WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1,
> +					seat->xkb_info.keymap_fd,
> +					seat->xkb_info.keymap_size);
> +	} else {
> +		wl_keyboard_send_keymap(cr, WL_KEYBOARD_KEYMAP_FORMAT_NO_KEYMAP,
> +					0,
> +					0);

0 is a legal fd, and probably even an open one. -1 should be
guaranteed an invalid fd, and is usually silently ignored. Can you
send -1 instead?

If you send 0, you will probably clone the stdin file descriptor of
the server, and send it to client. Did you check that clients will
close the fd regardless of the format or length?


Thanks,
pq