[PATCH libinput 15/26] tablet: expand the button mask to allow for BTN_LEFT, RIGHT, MIDDLE
Benjamin Tissoires
benjamin.tissoires at gmail.com
Tue Feb 24 09:16:28 PST 2015
On Tue, Feb 24, 2015 at 1:21 AM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> Expand the mask to fit KEY_CNT buttons, the mouse has LMR buttons and a few
> more, trying to squash the range is more error-prone than having the full key
> range instead.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> src/evdev-tablet.c | 82 +++++++++++++++++++++++++++++++++---------------------
> src/evdev-tablet.h | 2 +-
> 2 files changed, 51 insertions(+), 33 deletions(-)
>
> diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
> index dfea318..85bdcd7 100644
> --- a/src/evdev-tablet.c
> +++ b/src/evdev-tablet.c
> @@ -35,10 +35,37 @@
> #define tablet_unset_status(tablet_,s_) (tablet_)->status &= ~(s_)
> #define tablet_has_status(tablet_,s_) (!!((tablet_)->status & (s_)))
>
> -#define tablet_get_pressed_buttons(tablet_,field_) \
> - ((tablet_)->button_state.field_ & ~((tablet_)->prev_button_state.field_))
> -#define tablet_get_released_buttons(tablet_,field_) \
> - ((tablet_)->prev_button_state.field_ & ~((tablet_)->button_state.field_))
> +static inline void
> +tablet_get_pressed_buttons(struct tablet_dispatch *tablet,
> + unsigned char *buttons,
> + size_t nelem)
nitpicking, but it took me a while to understand the nelem semantic.
Especially when comparing it to sizeof(state->stylus_buttons).
how about having an actual unisgned...
> +{
> + size_t i;
> + const struct button_state *state = &tablet->button_state,
> + *prev_state = &tablet->prev_button_state;
> +
> + assert(nelem <= sizeof(state->stylus_buttons));
... and compare it to ARRAY_SIZE(state->stylus_buttons)?
> +
> + for (i = 0; i < nelem; i++)
> + buttons[i] = state->stylus_buttons[i] &
> + ~(prev_state->stylus_buttons[i]);
> +}
> +
> +static inline void
> +tablet_get_released_buttons(struct tablet_dispatch *tablet,
> + unsigned char *buttons,
> + size_t nelem)
Same remark.
> +{
> + size_t i;
> + const struct button_state *state = &tablet->button_state,
> + *prev_state = &tablet->prev_button_state;
> +
> + assert(nelem <= sizeof(state->stylus_buttons));
> +
> + for (i = 0; i < nelem; i++)
> + buttons[i] = prev_state->stylus_buttons[i] &
> + ~(state->stylus_buttons[i]);
> +}
>
> static int
> tablet_device_has_axis(struct tablet_dispatch *tablet,
> @@ -248,14 +275,11 @@ tablet_update_button(struct tablet_dispatch *tablet,
> uint32_t evcode,
> uint32_t enable)
> {
> - uint32_t button, *mask;
>
> - /* XXX: This really depends on the expected buttons fitting in the mask */
> if (evcode >= BTN_MISC && evcode <= BTN_TASK) {
> return;
> } else if (evcode >= BTN_TOUCH && evcode <= BTN_STYLUS2) {
> - mask = &tablet->button_state.stylus_buttons;
> - button = evcode - BTN_TOUCH;
> + /* noop */
> } else {
> log_info(tablet->device->base.seat->libinput,
> "Unhandled button %s (%#x)\n",
> @@ -263,13 +287,11 @@ tablet_update_button(struct tablet_dispatch *tablet,
> return;
> }
>
> - assert(button < 32);
> -
> if (enable) {
> - (*mask) |= 1 << button;
> + set_bit(tablet->button_state.stylus_buttons, evcode);
> tablet_set_status(tablet, TABLET_BUTTONS_PRESSED);
> } else {
> - (*mask) &= ~(1 << button);
> + clear_bit(tablet->button_state.stylus_buttons, evcode);
> tablet_set_status(tablet, TABLET_BUTTONS_RELEASED);
> }
> }
> @@ -452,28 +474,22 @@ tablet_notify_button_mask(struct tablet_dispatch *tablet,
> struct evdev_device *device,
> uint32_t time,
> struct libinput_tool *tool,
> - uint32_t buttons,
> - uint32_t button_base,
> + const unsigned char *buttons,
> + size_t buttons_size,
> enum libinput_button_state state)
> {
> struct libinput_device *base = &device->base;
> - int32_t num_button = 0;
> + size_t i;
>
> - while (buttons) {
> - int enabled;
> -
> - num_button++;
> - enabled = (buttons & 1);
> - buttons >>= 1;
> -
> - if (!enabled)
> + for (i = 0; i < buttons_size * 8; i++) {
nitpicking again, but the 8 should be something else :)
> + if (!bit_is_set(buttons, i))
> continue;
>
> tablet_notify_button(base,
> time,
> tool,
> tablet->axes,
> - num_button + button_base - 1,
> + i,
> state);
> }
> }
> @@ -485,21 +501,21 @@ tablet_notify_buttons(struct tablet_dispatch *tablet,
> struct libinput_tool *tool,
> enum libinput_button_state state)
> {
> - uint32_t stylus_buttons;
> + unsigned char buttons[sizeof(tablet->button_state.stylus_buttons)];
Can't we use an ARRAY_SIZE here instead?
>
> if (state == LIBINPUT_BUTTON_STATE_PRESSED)
> - stylus_buttons =
> - tablet_get_pressed_buttons(tablet, stylus_buttons);
> + tablet_get_pressed_buttons(tablet, buttons, sizeof(buttons));
ditto
> else
> - stylus_buttons =
> - tablet_get_released_buttons(tablet, stylus_buttons);
> + tablet_get_released_buttons(tablet,
> + buttons,
> + sizeof(buttons));
ditto
Sorry for nitpicking, but I think the current code relies too much on
the fact that we have unsigned chars. I don't see the reason why, but
if we ever switch to longs, it will be a pain to change later.
Cheers,
Benjamin
>
> tablet_notify_button_mask(tablet,
> device,
> time,
> tool,
> - stylus_buttons,
> - BTN_TOUCH,
> + buttons,
> + sizeof(buttons),
> state);
> }
>
> @@ -542,7 +558,9 @@ tablet_flush(struct tablet_dispatch *tablet,
>
> if (tablet_has_status(tablet, TABLET_TOOL_LEAVING_PROXIMITY)) {
> /* Release all stylus buttons */
> - tablet->button_state.stylus_buttons = 0;
> + memset(tablet->button_state.stylus_buttons,
> + 0,
> + sizeof(tablet->button_state.stylus_buttons));
> tablet_set_status(tablet, TABLET_BUTTONS_RELEASED);
> } else if (tablet_has_status(tablet, TABLET_AXES_UPDATED) ||
> tablet_has_status(tablet, TABLET_TOOL_ENTERING_PROXIMITY)) {
> diff --git a/src/evdev-tablet.h b/src/evdev-tablet.h
> index 6226d63..d2806b4 100644
> --- a/src/evdev-tablet.h
> +++ b/src/evdev-tablet.h
> @@ -41,7 +41,7 @@ enum tablet_status {
> };
>
> struct button_state {
> - uint32_t stylus_buttons; /* bitmask of evcode - BTN_TOUCH */
> + unsigned char stylus_buttons[NCHARS(KEY_CNT)];
> };
>
> struct tablet_dispatch {
> --
> 2.1.0
>
> _______________________________________________
> 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