[PATCH libinput 14/24] tablet: handle button events

[Peter Hutterer]

On Mon, Apr 21, 2014 at 07:11:23PM +0200, Carlos Garnacho wrote:
> This works for stylus tap and primary/secondary button, and pad
> buttons.
> 
> Signed-off-by: Carlos Garnacho <carlosg at gnome.org>
> ---
>  src/evdev-tablet.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/evdev-tablet.h |   3 ++
>  2 files changed, 107 insertions(+)
> 
> diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
> index fcd3396..0682563 100644
> --- a/src/evdev-tablet.c
> +++ b/src/evdev-tablet.c
> @@ -30,6 +30,11 @@
>  #define tablet_unset_status(tablet,s) (tablet->status &= ~(s));
>  #define tablet_has_status(tablet,s) (!!(tablet->status & s))
>  
> +#define tablet_get_enabled_buttons(tablet,field) \
> +	(tablet->state.field & ~(tablet->prev_state.field))
> +#define tablet_get_disabled_buttons(tablet,field) \
> +	(tablet->prev_state.field & ~(tablet->state.field))

this is me being manic about type checks these days - I'd prefer these to be
inlines. Also: enabled and disabled, especially in a button context has
multiple meanings, can we use "down" and "up" instead?

> +
>  static void
>  tablet_process_absolute(struct tablet_dispatch *tablet,
>  			struct evdev_device *device,
> @@ -68,6 +73,31 @@ tablet_update_tool(struct tablet_dispatch *tablet,
>  }
>  
>  static void
> +tablet_update_button(struct tablet_dispatch *tablet,
> +		     uint32_t evcode,
> +		     uint32_t enable)
> +{
> +	uint32_t button, *flags;
> +
> +	/* XXX: This really depends on the expected buttons fitting in the mask */
> +	if (evcode >= BTN_MISC && evcode <= BTN_TASK) {
> +		flags = &tablet->state.pad_buttons;
> +		button = evcode - BTN_MISC;
> +	} else if (evcode >= BTN_TOUCH && evcode <= BTN_STYLUS2) {
> +		flags = &tablet->state.stylus_buttons;
> +		button = evcode - BTN_TOUCH;
> +	} else {
> +		log_info("Unhandled button 0x%x\n", evcode);

best to use libevdev_event_code_get_name() here, together with the hex
code in case it's an undefined code.

> +		return;
> +	}
> +
> +	if (enable)
> +		(*flags) |= 1 << button;
> +	else
> +		(*flags) &= ~(1 << button);
> +}
> +
> +static void
>  tablet_process_key(struct tablet_dispatch *tablet,
>  		   struct evdev_device *device,
>  		   struct input_event *e,
> @@ -85,7 +115,18 @@ tablet_process_key(struct tablet_dispatch *tablet,
>  		/* These codes have an equivalent libinput_tool value */
>  		tablet_update_tool(tablet, e->code, e->value);
>  		break;
> +	case BTN_TOUCH:
> +		if (e->value) {
> +			tablet_set_status(tablet, TABLET_HAS_CONTACT);
> +		} else {
> +			tablet_unset_status(tablet, TABLET_HAS_CONTACT);
> +		}

please don't use {} for single-line statements (provided both are
single-line)

> +
> +		/* Fall through */
> +	case BTN_STYLUS:
> +	case BTN_STYLUS2:
>  	default:
> +		tablet_update_button(tablet, e->code, e->value);
>  		break;
>  	}
>  }
> @@ -131,6 +172,67 @@ tablet_check_notify_tool(struct tablet_dispatch *tablet,
>  }
>  
>  static void
> +tablet_notify_button_mask(struct tablet_dispatch *tablet,
> +			  struct evdev_device *device,
> +			  uint32_t time,
> +			  uint32_t buttons,
> +			  uint32_t button_base,
> +			  enum libinput_pointer_button_state state)
> +{
> +	struct libinput_device *base = &device->base;
> +	int32_t num_button = 0;
> +
> +	while (buttons) {
> +		int enabled;
> +
> +		num_button++;
> +		enabled = (buttons & 1);
> +		buttons >>= 1;
> +
> +		if (!enabled)
> +			continue;
> +
> +		pointer_notify_button(base,
> +				      time,
> +				      num_button + button_base - 1,
> +				      state);
> +	}
> +}
> +
> +static void
> +tablet_notify_buttons(struct tablet_dispatch *tablet,
> +		      struct evdev_device *device,
> +		      uint32_t time,
> +		      uint32_t post_check)

use bool for post_check

> +{
> +	enum libinput_pointer_button_state state;
> +	int32_t pad_buttons, stylus_buttons;
> +
> +	if (tablet->state.pad_buttons == tablet->prev_state.pad_buttons &&
> +	    tablet->state.stylus_buttons == tablet->prev_state.stylus_buttons)
> +		return;
> +
> +	if (post_check) {
> +		/* Only notify button releases */
> +		state = LIBINPUT_POINTER_BUTTON_STATE_RELEASED;
> +		pad_buttons = tablet_get_disabled_buttons(tablet, pad_buttons);
> +		stylus_buttons =
> +			tablet_get_disabled_buttons(tablet, stylus_buttons);
> +	} else {
> +		/* Only notify button presses */
> +		state = LIBINPUT_POINTER_BUTTON_STATE_PRESSED;
> +		pad_buttons = tablet_get_enabled_buttons(tablet, pad_buttons);
> +		stylus_buttons =
> +			tablet_get_enabled_buttons(tablet, stylus_buttons);
> +	}
> +
> +	tablet_notify_button_mask(
> +		tablet, device, time, pad_buttons, BTN_MISC, state);
> +	tablet_notify_button_mask(
> +		tablet, device, time, stylus_buttons, BTN_TOUCH, state);

yikes. I really dislike this formatting style. We generally leave the first
parameter in the same line, with the rest indented to match.
For this case, you could group it like this:

        tablet_notify_button_mask(tablet, device, time,
                                  pad_buttons, BTN_MISC, state);

> +}
> +
> +static void
>  tablet_flush(struct tablet_dispatch *tablet,
>  	     struct evdev_device *device,
>  	     uint32_t time)
> @@ -140,6 +242,7 @@ tablet_flush(struct tablet_dispatch *tablet,
>  
>  	/* pre-update notifications */
>  	tablet_check_notify_tool(tablet, device, time, 0);
> +	tablet_notify_buttons(tablet, device, time, 0);
>  
>  	if (tablet->state.tool != LIBINPUT_TOOL_NONE) {
>  		if (tablet_has_status(tablet, TABLET_UPDATED)) {
> @@ -153,6 +256,7 @@ tablet_flush(struct tablet_dispatch *tablet,
>  	}
>  
>  	/* post-update notifications */
> +	tablet_notify_buttons(tablet, device, time, 1);
>  	tablet_check_notify_tool(tablet, device, time, 1);
>  
>  	/* replace previous state */
> diff --git a/src/evdev-tablet.h b/src/evdev-tablet.h
> index 7b7b066..d81c878 100644
> --- a/src/evdev-tablet.h
> +++ b/src/evdev-tablet.h
> @@ -30,9 +30,12 @@ enum tablet_status {
>  	TABLET_NONE = 0,
>  	TABLET_UPDATED = 1 << 0,
>  	TABLET_INTERACTED = 1 << 1,
> +	TABLET_HAS_CONTACT = 1 << 2,

this should be TABLET_STYLUS_IN_CONTACT, you don't get BTN_TOUCH for pure
button presses.

Cheers,
   Peter

>  };
>  
>  struct device_state {
> +	uint32_t pad_buttons; /* bitmask of evcode - BTN_MISC */
> +	uint32_t stylus_buttons; /* bitmask of evcode - BTN_TOUCH */
>  	enum libinput_tool tool;
>  	int32_t tool_serial;
>  };
> -- 
> 1.9.0