[PATCH evdev 7/8] Store per-slot touch state

Peter Hutterer peter.hutterer at who-t.net
Wed Jan 9 15:55:45 PST 2013


On Wed, Jan 09, 2013 at 07:21:21PM +0100, Benjamin Tissoires wrote:
> The previous implementation considers that a new slot means a TouchBegin,
> and the deletion of the slot a TouchEnd. However, this is not compliant
> with the mt evdev specification as the touch state is given by
> ABS_MT_DISTANCE == 0.
> Before Win 8 specification, no known multitouch devices present in
> proximity events (like the hovering of Wacom stylus). But since this
> specification exists, there may be devices exposing ABS_MT_DISTANCE.
> 
> The modification here consists in storing the current touch state of
> each slot, to be able to detect if we need to forward Begin/End/Update
> event by just setting the touch property to true or false.
> 
> The benefit of it is that we can now easily add proximity support
> and several touch begin/end within the same slot.
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires at gmail.com>
> ---
>  src/evdev.c | 47 +++++++++++++++++++++++++++++++----------------
>  src/evdev.h |  8 +++++++-
>  2 files changed, 38 insertions(+), 17 deletions(-)
> 
> diff --git a/src/evdev.c b/src/evdev.c
> index 8d7853b..256037c 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -736,21 +736,33 @@ EvdevProcessTouch(InputInfoPtr pInfo)
>      EvdevPtr pEvdev = pInfo->private;
>      int type;
>      int slot_index = last_mt_vals_slot(pEvdev);
> +    EvdevMTState *mt_state;
>  
>      if (slot_index < 0 || !pEvdev->mt_mask)
>          return;
>  
> +    mt_state = &pEvdev->mt_state[slot_index];
> +
>      /* If the ABS_MT_SLOT is the first event we get after EV_SYN, skip this */
>      if (pEvdev->slot_state == SLOTSTATE_EMPTY)
>          return;
>  
>      if (pEvdev->slot_state == SLOTSTATE_CLOSE)
> -        type = XI_TouchEnd;
> +        mt_state->touch = FALSE;
>      else if (pEvdev->slot_state == SLOTSTATE_OPEN)
> -        type = XI_TouchBegin;
> -    else
> -        type = XI_TouchUpdate;
> +        mt_state->touch = TRUE;
>  
> +    type = XI_TouchUpdate;
> +
> +    if (mt_state->touch != mt_state->touch_state) {
> +        /* We are getting in/out of touch */
> +        if (mt_state->touch)
> +            type = XI_TouchBegin;
> +        else
> +            type = XI_TouchEnd;
> +
> +        mt_state->touch_state = mt_state->touch;
> +    }
>  
>      EvdevQueueTouchEvent(pInfo, slot_index, pEvdev->mt_mask, type);
>  
> @@ -794,6 +806,7 @@ EvdevProcessTouchEvent(InputInfoPtr pInfo, struct input_event *ev)
>  {
>      EvdevPtr pEvdev = pInfo->private;
>      int map;
> +    EvdevMTState *mt_state;
>  
>      if (!EvdevIsMTDevice(pEvdev))
>          return;
> @@ -813,19 +826,21 @@ EvdevProcessTouchEvent(InputInfoPtr pInfo, struct input_event *ev)
>              return;
>          }
>  
> +        mt_state = &pEvdev->mt_state[slot_index];
> +
>          if (pEvdev->slot_state == SLOTSTATE_EMPTY)
>              pEvdev->slot_state = SLOTSTATE_UPDATE;
>          if (ev->code == ABS_MT_TRACKING_ID) {
>              if (ev->value >= 0) {
>                  pEvdev->slot_state = SLOTSTATE_OPEN;
>                  valuator_mask_copy(pEvdev->mt_mask,
> -                                   pEvdev->last_mt_vals[slot_index]);
> +                                   mt_state->last_vals);
>              } else
>                  pEvdev->slot_state = SLOTSTATE_CLOSE;
>          } else {
>              map = pEvdev->axis_map[ev->code];
>              valuator_mask_set(pEvdev->mt_mask, map, ev->value);
> -            valuator_mask_set(pEvdev->last_mt_vals[slot_index], map, ev->value);
> +            valuator_mask_set(mt_state->last_vals, map, ev->value);
>          }
>      }
>  }
> @@ -1098,12 +1113,12 @@ EvdevFreeMasks(EvdevPtr pEvdev)
>      valuator_mask_free(&pEvdev->prox);
>  #ifdef MULTITOUCH
>      valuator_mask_free(&pEvdev->mt_mask);
> -    if (pEvdev->last_mt_vals)
> +    if (pEvdev->mt_state)
>      {
>          for (i = 0; i < num_slots(pEvdev); i++)
> -            valuator_mask_free(&pEvdev->last_mt_vals[i]);
> -        free(pEvdev->last_mt_vals);
> -        pEvdev->last_mt_vals = NULL;
> +            valuator_mask_free(&pEvdev->mt_state[i].last_vals);
> +        free(pEvdev->mt_state);
> +        pEvdev->mt_state = NULL;
>      }
>      for (i = 0; i < EVDEV_MAXQUEUE; i++)
>          valuator_mask_free(&pEvdev->queue[i].touchMask);
> @@ -1333,17 +1348,17 @@ EvdevAddAbsValuatorClass(DeviceIntPtr device)
>              goto out;
>          }
>  
> -        pEvdev->last_mt_vals = calloc(num_slots(pEvdev), sizeof(ValuatorMask *));
> -        if (!pEvdev->last_mt_vals) {
> +        pEvdev->mt_state = calloc(num_slots(pEvdev), sizeof(EvdevMTState));
> +        if (!pEvdev->mt_state) {
>              xf86IDrvMsg(pInfo, X_ERROR,
> -                        "%s: failed to allocate MT last values mask array.\n",
> +                        "%s: failed to allocate MT state.\n",
>                          device->name);
>              goto out;
>          }
>  
>          for (i = 0; i < num_slots(pEvdev); i++) {
> -            pEvdev->last_mt_vals[i] = valuator_mask_new(num_mt_axes_total);
> -            if (!pEvdev->last_mt_vals[i]) {
> +            pEvdev->mt_state[i].last_vals = valuator_mask_new(num_mt_axes_total);
> +            if (!pEvdev->mt_state[i].last_vals) {
>                  xf86IDrvMsg(pInfo, X_ERROR,
>                              "%s: failed to allocate MT last values mask.\n",
>                              device->name);
> @@ -1439,7 +1454,7 @@ EvdevAddAbsValuatorClass(DeviceIntPtr device)
>                                      strerror(errno));
>                  }
>                  for (i = 0; i < num_touches; i++)
> -                    valuator_mask_set(pEvdev->last_mt_vals[i],
> +                    valuator_mask_set(pEvdev->mt_state[i].last_vals,
>                                        pEvdev->axis_map[axis],
>                                        mt_request_data[i]);
>              }
> diff --git a/src/evdev.h b/src/evdev.h
> index 2901886..6195c79 100644
> --- a/src/evdev.h
> +++ b/src/evdev.h
> @@ -150,6 +150,12 @@ typedef struct {
>  } EventQueueRec, *EventQueuePtr;
>  
>  typedef struct {
> +    ValuatorMask *last_vals;
> +    int touch;                /* per-slot incoming touch state */
> +    int touch_state;          /* per-slot current touch state */

this is quite a confusing variable naming, without the comments it's
impossible to decipher which does which. and even with the comment i'm not
sure... can we rename these to old_touch_state and touch_state, this is a convention
that we use in general.

or can we add new slot states and make this one single enum? I'm thinking of 
something like SLOTSTATE_OPEN, SLOTSTATE_IN_TOUCH, SLOTSTATE_CLOSE. it'll
require more state transition handling than a simple != but I think it'll be
more readable.

this patch also needs to add a comment to slot_state to clarify that this is
the _current_ state only.

Cheers,
   Peter


> +} EvdevMTState;
> +
> +typedef struct {
>      unsigned short id_vendor;
>      unsigned short id_product;
>  
> @@ -163,11 +169,11 @@ typedef struct {
>      ValuatorMask *old_vals; /* old values for calculating relative motion */
>      ValuatorMask *prox;     /* last values set while not in proximity */
>      ValuatorMask *mt_mask;
> -    ValuatorMask **last_mt_vals;
>      int cur_slot;
>      enum SlotState slot_state;
>  #ifdef MULTITOUCH
>      struct mtdev *mtdev;
> +    EvdevMTState *mt_state;
>  #endif
>  
>      int flags;
> -- 
> 1.8.0.2
> 


More information about the xorg-devel mailing list