[PATCH weston 06/31] Do binding modifier lookup on XKB state, not physical keys

[Kristian Høgsberg]

On Wed, May 30, 2012 at 04:31:44PM +0100, Daniel Stone wrote:
> When we update the modifier_state used for Weston bindings, derive this
> from the XKB modifier state, rather than a hardcoded mapping of physical
> keys to modifier state.
> 
> Signed-off-by: Daniel Stone <daniel at fooishbar.org>
> ---
>  src/compositor.c |   46 ++++++++++++++++++++--------------------------
>  src/compositor.h |    3 +++
>  2 files changed, 23 insertions(+), 26 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index f4f5962..e310ceb 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -1733,13 +1733,16 @@ notify_axis(struct wl_seat *seat, uint32_t time, uint32_t axis, int32_t value)
>  static int
>  update_modifier_state(struct weston_seat *seat, uint32_t key, uint32_t state)
>  {
> -	enum weston_keyboard_modifier modifier;
>  	uint32_t mods_depressed, mods_latched, mods_locked, group;
> +	uint32_t mods_lookup;
>  	int ret = 0;
>  
> +	/* First update the XKB state object with the keypress. */
>  	xkb_state_update_key(seat->xkb_state.state, key + 8,
>  			     state ? XKB_KEY_DOWN : XKB_KEY_UP);
>  
> +	/* Serialize and update our internal state, checking to see if it's
> +	 * different to the previous state. */
>  	mods_depressed = xkb_state_serialize_mods(seat->xkb_state.state,
>  						  XKB_STATE_DEPRESSED);
>  	mods_latched = xkb_state_serialize_mods(seat->xkb_state.state,
> @@ -1760,31 +1763,15 @@ update_modifier_state(struct weston_seat *seat, uint32_t key, uint32_t state)
>  	seat->xkb_state.mods_locked = mods_locked;
>  	seat->xkb_state.group = group;
>  
> -	switch (key) {
> -	case KEY_LEFTCTRL:
> -	case KEY_RIGHTCTRL:
> -		modifier = MODIFIER_CTRL;
> -		break;
> -
> -	case KEY_LEFTALT:
> -	case KEY_RIGHTALT:
> -		modifier = MODIFIER_ALT;
> -		break;
> -
> -	case KEY_LEFTMETA:
> -	case KEY_RIGHTMETA:
> -		modifier = MODIFIER_SUPER;
> -		break;
> -
> -	default:
> -		modifier = 0;
> -		break;
> -	}
> -
> -	if (state)
> -		seat->modifier_state |= modifier;
> -	else
> -		seat->modifier_state &= ~modifier;
> +	/* And update the modifier_state for bindings. */
> +	mods_lookup = mods_depressed | mods_latched;
> +	seat->modifier_state = 0;
> +	if ((mods_lookup & seat->compositor->xkb_info.ctrl_mod))
> +		seat->modifier_state |= MODIFIER_CTRL;
> +	if ((mods_lookup & seat->compositor->xkb_info.alt_mod))
> +		seat->modifier_state |= MODIFIER_ALT;
> +	if ((mods_lookup & seat->compositor->xkb_info.super_mod))
> +		seat->modifier_state |= MODIFIER_SUPER;
>  
>  	return ret;
>  }
> @@ -2245,6 +2232,13 @@ static int weston_compositor_xkb_init(struct weston_compositor *ec,
>  		return -1;
>  	}
>  
> +	ec->xkb_info.ctrl_mod = xkb_map_mod_get_index(ec->xkb_info.keymap,
> +						      XKB_MOD_NAME_CTRL);

The patch is good and committed as is, but I just want to tout my
preferred line breaking style since this is a good case.  In the above
you're breaking the line in the middle of the arguments, which is
pretty common, but I think this is so much more readable:

	ec->xkb_info.ctrl_mod =
		xkb_map_mod_get_index(ec->xkb_info.keymap, XKB_MOD_NAME_CTRL);

since it closer follows how the elements on the line naturally group
and is still just two lines total.  Instead of breaking up the
function call (higher operator precedence) we break at the assignment
(lower operator precedence) and avoid breaking up the assignment
expression subtrees.

Kristian

> +	ec->xkb_info.alt_mod = xkb_map_mod_get_index(ec->xkb_info.keymap,
> +						     XKB_MOD_NAME_ALT);
> +	ec->xkb_info.super_mod = xkb_map_mod_get_index(ec->xkb_info.keymap,
> +						       XKB_MOD_NAME_LOGO);
> +
>  	return 0;
>  }
>  
> diff --git a/src/compositor.h b/src/compositor.h
> index 08086f7..57a49de 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -300,6 +300,9 @@ struct weston_compositor {
>  		struct xkb_rule_names names;
>  		struct xkb_context *context;
>  		struct xkb_keymap *keymap;
> +		xkb_mod_index_t ctrl_mod;
> +		xkb_mod_index_t alt_mod;
> +		xkb_mod_index_t super_mod;
>  	} xkb_info;
>  };
>  
> -- 
> 1.7.10
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel