[PATCH 1/4] Port Wayland clients to new xkbcommon API

Kristian Høgsberg hoegsberg at gmail.com
Mon May 7 13:01:16 PDT 2012


On Tue, May 01, 2012 at 08:37:09PM +0100, Daniel Stone wrote:
> Signed-off-by: Daniel Stone <daniel at fooishbar.org>
> ---
>  clients/eventdemo.c |    5 ++-
>  clients/terminal.c  |   33 ++++++++++--------
>  clients/window.c    |   96 +++++++++++++++++++++++++++++++--------------------
>  clients/window.h    |    9 +++--
>  4 files changed, 88 insertions(+), 55 deletions(-)
> 
> diff --git a/clients/eventdemo.c b/clients/eventdemo.c
> index e7904e6..8937dce 100644
> --- a/clients/eventdemo.c
> +++ b/clients/eventdemo.c
> @@ -33,6 +33,7 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  
> +#include <xkbcommon/xkbcommon.h>
>  #include <cairo.h>
>  #include <glib.h>
>  
> @@ -191,7 +192,9 @@ static void
>  key_handler(struct window *window, struct input *input, uint32_t time,
>              uint32_t key, uint32_t unicode, uint32_t is_down, void *data)
>  {
> -	uint32_t modifiers = input_get_modifiers(input);
> +	struct xkb_state *xkb_state = input_get_xkb_state(input);
> +	uint32_t modifiers = xkb_state_serialise_mods(xkb_state,
> +						      XKB_STATE_EFFECTIVE);
>  
>  	if(!log_key)
>  		return;
> diff --git a/clients/terminal.c b/clients/terminal.c
> index 57e8030..99b3275 100644
> --- a/clients/terminal.c
> +++ b/clients/terminal.c
> @@ -35,6 +35,7 @@
>  #include <sys/epoll.h>
>  
>  #include <X11/keysym.h>
> +#include <xkbcommon/xkbcommon.h>
>  
>  #include <wayland-client.h>
>  
> @@ -266,16 +267,11 @@ static struct key_map KM_APPLICATION[] = {
>  };
>  
>  static int
> -function_key_response(char escape, int num, uint32_t modifiers,
> +function_key_response(char escape, int num, uint32_t mod_num,
>  		      char code, char *response)
>  {
> -	int mod_num = 0;
>  	int len;
>  
> -	if (modifiers & XKB_COMMON_SHIFT_MASK) mod_num   |= 1;
> -	if (modifiers & XKB_COMMON_MOD1_MASK) mod_num    |= 2;
> -	if (modifiers & XKB_COMMON_CONTROL_MASK) mod_num |= 4;
> -
>  	if (mod_num != 0)
>  		len = snprintf(response, MAX_RESPONSE, "\e[%d;%d%c",
>  			       num, mod_num + 1, code);
> @@ -2074,12 +2070,21 @@ key_handler(struct window *window, struct input *input, uint32_t time,
>  {
>  	struct terminal *terminal = data;
>  	char ch[MAX_RESPONSE];
> -	uint32_t modifiers;
> +	uint32_t modifiers = 0;
>  	int len = 0;
> -
> -	modifiers = input_get_modifiers(input);
> -	if ((modifiers & XKB_COMMON_CONTROL_MASK) &&
> -	    (modifiers & XKB_COMMON_SHIFT_MASK) &&
> +	struct xkb_state *xkb_state = input_get_xkb_state(input);
> +
> +	if (xkb_state_mod_name_is_active(xkb_state, "Shift",
> +	                                 (XKB_STATE_DEPRESSED | XKB_STATE_LATCHED)))
> +		modifiers |= MOD_SHIFT;
> +	if (xkb_state_mod_name_is_active(xkb_state, "Control",
> +	                                 (XKB_STATE_DEPRESSED | XKB_STATE_LATCHED)))
> +		modifiers |= MOD_CTRL;
> +	if (xkb_state_mod_name_is_active(xkb_state, "Mod1",
> +	                                 (XKB_STATE_DEPRESSED | XKB_STATE_LATCHED)))
> +		modifiers |= MOD_ALT;
> +
> +	if ((modifiers & MOD_CTRL) && (modifiers & MOD_SHIFT) &&

I'm still not convinced this API is practical.  Testing a specific
combination of modifiers is such a common operation and it just can't
be this awkward.  The original code tests that shift and control are
pressed, but it really should test modifers == SHIFT | CONTROL, that
is, that those and *only* those modifiers are down.  To do that we
need to get the modifier masks up front (in display_create for
window.c), and then create the modifier mask we want to test against:

  mask = display_get_shift_mask(display) | display_get_control_mask(display);

and then say

  mods = xkb_state_serialise_mods(state, XKB_STATE_DEPRESSED | XKB_STATE_LATCHED);

  if (mask == mods)
    ...

or we can define MOD_CTRL in window.h and then translate the
serialized mask to those values:

  if (input_get_modifiers(input) == (MOD_CTRL | MOD_SHIFT))
    ...

which is what we had before.  Plus a MOD_OTHER if something else we
don't know about is down.  So maybe it's ok if we just wrap it in the
toolkits...  At the end of the day, we'll just hard code shift, ctrl
and alt again, just a layer up in the toolkits.

>  	    is_down && handle_bound_key(terminal, input, sym, time))
>  		return;
>  
> @@ -2173,7 +2178,7 @@ key_handler(struct window *window, struct input *input, uint32_t time,
>  		len = apply_key_map(terminal->key_mode, sym, modifiers, ch);
>  		if (len != 0) break;
>  		
> -		if (modifiers & XKB_COMMON_CONTROL_MASK) {
> +		if (modifiers & MOD_CTRL) {
>  			if (sym >= '3' && sym <= '7')
>  				sym = (sym & 0x1f) + 8;
>  
> @@ -2184,10 +2189,10 @@ key_handler(struct window *window, struct input *input, uint32_t time,
>  			else if (sym == '/') sym = 0x1F;
>  			else if (sym == '8' || sym == '?') sym = 0x7F;
>  		} else if ((terminal->mode & MODE_ALT_SENDS_ESC) && 
> -			   (modifiers & XKB_COMMON_MOD1_MASK))
> +			   (modifiers & MOD_ALT))
>  		{
>  			ch[len++] = 0x1b;
> -		} else if (modifiers & XKB_COMMON_MOD1_MASK) {
> +		} else if (modifiers & MOD_ALT) {
>  			sym = sym | 0x80;
>  		}
>  
> diff --git a/clients/window.c b/clients/window.c
> index 831f061..99c54f4 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -90,7 +90,12 @@ struct display {
>  	struct wl_list output_list;
>  	cairo_surface_t *active_frame, *inactive_frame, *shadow;
>  	int frame_radius;
> -	struct xkb_desc *xkb;
> +	struct {
> +		struct xkb_rule_names names;
> +		struct xkb_keymap *keymap;
> +		struct xkb_state *state;
> +		struct xkb_context *context;
> +	} xkb_info;
>  	cairo_surface_t **pointer_surfaces;
>  
>  	PFNGLEGLIMAGETARGETTEXTURE2DOESPROC image_target_texture_2d;
> @@ -173,7 +178,6 @@ struct input {
>  	struct window *pointer_focus;
>  	struct window *keyboard_focus;
>  	int current_pointer_image;
> -	uint32_t modifiers;
>  	uint32_t pointer_enter_serial;
>  	int32_t sx, sy;
>  	struct wl_list link;
> @@ -1402,30 +1406,30 @@ input_handle_key(void *data, struct wl_input_device *input_device,
>  	struct input *input = data;
>  	struct window *window = input->keyboard_focus;
>  	struct display *d = input->display;
> -	uint32_t code, sym, level;
> +	uint32_t code, num_syms;
> +	const xkb_keysym_t *syms;
>  
>  	input->display->serial = serial;
>  	code = key + 8;
>  	if (!window || window->keyboard_device != input)
>  		return;
>  
> -	level = 0;
> -	if (input->modifiers & XKB_COMMON_SHIFT_MASK &&
> -	    XkbKeyGroupWidth(d->xkb, code, 0) > 1)
> -		level = 1;
> +	num_syms = xkb_key_get_syms(d->xkb_info.state, code, &syms);
> +	xkb_state_update_key(d->xkb_info.state, code,
> +			     is_down ? XKB_KEY_DOWN : XKB_KEY_UP);
>  
> -	sym = XkbKeySymEntry(d->xkb, code, level, 0);
> -
> -	if (is_down)
> -		input->modifiers |= d->xkb->map->modmap[code];
> -	else
> -		input->modifiers &= ~d->xkb->map->modmap[code];
> -
> -	if (key == KEY_F5 && input->modifiers == Mod4Mask) {
> +	if (num_syms == 0 && syms[0] == XK_F5 &&

Should this num_syms == 1 here?

> +	    xkb_state_mod_name_is_active(d->xkb_info.state, "Mod4",
> +					 XKB_STATE_EFFECTIVE)) {

Here's another case where we don't just want to test if Mod4 is down,
but if that it's the only modifier down.

>  		if (is_down)
>  			window_set_maximized(window,
>  					     window->type != TYPE_MAXIMIZED);
>  	} else if (window->key_handler) {
> +		xkb_keysym_t sym;
> +		if (num_syms != 1)
> +			sym = NoSymbol;
> +		else
> +			sym = syms[0];
>  		(*window->key_handler)(window, input, time, key,
>  				       sym, is_down, window->user_data);
>  	}
> @@ -1511,17 +1515,10 @@ input_handle_keyboard_enter(void *data,
>  {
>  	struct input *input = data;
>  	struct window *window;
> -	struct display *d = input->display;
> -	uint32_t *k, *end;
>  
>  	input->display->serial = serial;
>  	input->keyboard_focus = wl_surface_get_user_data(surface);
>  
> -	end = keys->data + keys->size;
> -	input->modifiers = 0;
> -	for (k = keys->data; k < end; k++)
> -		input->modifiers |= d->xkb->map->modmap[*k];
> -

This part break setting modifier state on keyboard enter, but the
planned keyboard.modifiers events will restore that and give us
modifier state for pointer enter as well.

>  	window = input->keyboard_focus;
>  	window->keyboard_device = input;
>  	if (window->keyboard_focus_handler)
> @@ -1606,18 +1603,24 @@ input_get_input_device(struct input *input)
>  	return input->input_device;
>  }
>  
> -uint32_t
> -input_get_modifiers(struct input *input)
> -{
> -	return input->modifiers;
> -}
> -
>  struct widget *
>  input_get_focus_widget(struct input *input)
>  {
>  	return input->focus_widget;
>  }
>  
> +struct xkb_state *
> +input_get_xkb_state(struct input *input)
> +{
> +	return input->display->xkb_info.state;
> +}
> +
> +struct xkb_keymap *
> +input_get_xkb_keymap(struct input *input)
> +{
> +	return input->display->xkb_info.keymap;
> +}
> +
>  struct data_offer {
>  	struct wl_data_offer *offer;
>  	struct input *input;
> @@ -2674,25 +2677,44 @@ display_render_frame(struct display *d)
>  static void
>  init_xkb(struct display *d)
>  {
> -	struct xkb_rule_names names;
> +	d->xkb_info.names.rules = strdup("evdev");
> +	d->xkb_info.names.model = strdup("pc105");
> +	d->xkb_info.names.layout = strdup(option_xkb_layout);
> +	d->xkb_info.names.variant = strdup(option_xkb_variant);
> +	d->xkb_info.names.options = strdup(option_xkb_options);
>  
> -	names.rules = "evdev";
> -	names.model = "pc105";
> -	names.layout = option_xkb_layout;
> -	names.variant = option_xkb_variant;
> -	names.options = option_xkb_options;

Is there anything in the libxkbcommon API changes that requires these
strings to be strdup'ed?

> +	d->xkb_info.context = xkb_context_new();
> +	if (!d->xkb_info.context) {
> +		fprintf(stderr, "Failed to create XKB context\n");
> +		exit(1);
> +	}
>  
> -	d->xkb = xkb_compile_keymap_from_rules(&names);
> -	if (!d->xkb) {
> +	d->xkb_info.keymap = xkb_map_new_from_names(d->xkb_info.context,
> +						    &d->xkb_info.names);
> +	if (!d->xkb_info.keymap) {
>  		fprintf(stderr, "Failed to compile keymap\n");
>  		exit(1);
>  	}
> +
> +	d->xkb_info.state = xkb_state_new(d->xkb_info.keymap);
> +	if (!d->xkb_info.state) {
> +		fprintf(stderr, "Failed to create XKB state\n");
> +		exit(1);
> +	}
>  }
>  
>  static void
>  fini_xkb(struct display *display)
>  {
> -	xkb_free_keymap(display->xkb);
> +	xkb_state_unref(display->xkb_info.state);
> +	xkb_map_unref(display->xkb_info.keymap);
> +	xkb_context_unref(display->xkb_info.context);
> +
> +	free(display->xkb_info.names.rules);
> +	free(display->xkb_info.names.model);
> +	free(display->xkb_info.names.layout);
> +	free(display->xkb_info.names.variant);
> +	free(display->xkb_info.names.options);
>  }
>  
>  #ifdef HAVE_CAIRO_EGL
> diff --git a/clients/window.h b/clients/window.h
> index 6bdffce..4bf45d6 100644
> --- a/clients/window.h
> +++ b/clients/window.h
> @@ -358,15 +358,18 @@ input_set_pointer_image(struct input *input, uint32_t time, int pointer);
>  void
>  input_get_position(struct input *input, int32_t *x, int32_t *y);
>  
> -uint32_t
> -input_get_modifiers(struct input *input);
> -
>  void
>  input_grab(struct input *input, struct widget *widget, uint32_t button);
>  
>  void
>  input_ungrab(struct input *input);
>  
> +struct xkb_keymap *
> +input_get_xkb_keymap(struct input *input);
> +
> +struct xkb_state *
> +input_get_xkb_state(struct input *input);
> +
>  struct widget *
>  input_get_focus_widget(struct input *input);
>  
> -- 
> 1.7.10
> 
> _______________________________________________
> 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