[PATCH v2 evdev] Make the slot-state per slot
Peter Hutterer
peter.hutterer at who-t.net
Sun Aug 17 17:03:18 PDT 2014
On Fri, Aug 15, 2014 at 09:14:49AM +0200, walter harms wrote:
> thx for taking me serious
> much better to read now.
> It is to see that SLOTSTATE_UPDATE is not covered by any case.
> I assume that is intentional ?
yeah, the default case covers it. but I've added it now so it's more
obvious. Technically the default case can be dumped now or could trigger a
BUG_ macro but I'll skip that for now.
thanks for the review
Cheers,
Peter
> revieded-by wharms <wharms at bfs.de>
> Am 15.08.2014 05:57, schrieb Peter Hutterer:
> > The previous approach only had the slot state for the current slot. If we
> > changed slots, that means we lost the information if the slot was ever
> > initialized. If the ABS_MT_TRACKING_ID was never received, the slot would
> > still update and try to send events (which the server refused with a warning).
> >
> > Avoid this by having a per-slot state and a dirty bit that tells us if the
> > current slot updated at all. If we don't get the tracking ID, leave the slot
> > empty and refuse any further events from that touch.
> >
> > This quashes the various "unable to find touch point 0" warnings caused if a
> > touchpoint starts before the device is enabled.
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> > Changes to v1:
> > - replace the convoluted if/else with a switch
> >
> > src/evdev.c | 75 +++++++++++++++++++++++++++++++++++++++----------------------
> > src/evdev.h | 5 ++++-
> > 2 files changed, 52 insertions(+), 28 deletions(-)
> >
> > diff --git a/src/evdev.c b/src/evdev.c
> > index 30f809b..4eebced 100644
> > --- a/src/evdev.c
> > +++ b/src/evdev.c
> > @@ -687,28 +687,37 @@ EvdevProcessTouch(InputInfoPtr pInfo)
> > {
> > EvdevPtr pEvdev = pInfo->private;
> > int type;
> > + int slot = pEvdev->cur_slot;
> >
> > - if (pEvdev->cur_slot < 0 || !pEvdev->mt_mask)
> > + if (slot < 0 || !pEvdev->mt_mask)
> > return;
> >
> > - /* If the ABS_MT_SLOT is the first event we get after EV_SYN, skip this */
> > - if (pEvdev->slot_state == SLOTSTATE_EMPTY)
> > + if (!pEvdev->slots[slot].dirty)
> > return;
> >
> > - if (pEvdev->slot_state == SLOTSTATE_CLOSE)
> > - type = XI_TouchEnd;
> > - else if (pEvdev->slot_state == SLOTSTATE_OPEN)
> > - type = XI_TouchBegin;
> > - else
> > - type = XI_TouchUpdate;
> > -
> > + switch(pEvdev->slots[slot].state)
> > + {
> > + case SLOTSTATE_EMPTY:
> > + return;
> > + case SLOTSTATE_CLOSE:
> > + type = XI_TouchEnd;
> > + pEvdev->slots[slot].state = SLOTSTATE_EMPTY;
> > + break;
> > + case SLOTSTATE_OPEN:
> > + type = XI_TouchBegin;
> > + pEvdev->slots[slot].state = SLOTSTATE_UPDATE;
> > + break;
> > + default:
> > + type = XI_TouchUpdate;
> > + break;
> > + }
> >
> > EvdevSwapAbsValuators(pEvdev, pEvdev->mt_mask);
> > EvdevApplyCalibration(pEvdev, pEvdev->mt_mask);
> >
> > EvdevQueueTouchEvent(pInfo, pEvdev->cur_slot, pEvdev->mt_mask, type);
> >
> > - pEvdev->slot_state = SLOTSTATE_EMPTY;
> > + pEvdev->slots[slot].dirty = 0;
> >
> > valuator_mask_zero(pEvdev->mt_mask);
> > }
> > @@ -751,29 +760,28 @@ EvdevProcessTouchEvent(InputInfoPtr pInfo, struct input_event *ev)
> > } else
> > {
> > int slot_index = last_mt_vals_slot(pEvdev);
> > + if (slot_index < 0) {
> > + LogMessageVerbSigSafe(X_WARNING, 0,
> > + "%s: Invalid slot index %d, touch events may be incorrect.\n",
> > + pInfo->name,
> > + slot_index);
> > + return;
> > + }
> >
> > - if (pEvdev->slot_state == SLOTSTATE_EMPTY)
> > - pEvdev->slot_state = SLOTSTATE_UPDATE;
> > + pEvdev->slots[slot_index].dirty = 1;
> > if (ev->code == ABS_MT_TRACKING_ID) {
> > if (ev->value >= 0) {
> > - pEvdev->slot_state = SLOTSTATE_OPEN;
> > + pEvdev->slots[slot_index].state = SLOTSTATE_OPEN;
> >
> > - if (slot_index >= 0)
> > - valuator_mask_copy(pEvdev->mt_mask,
> > - pEvdev->last_mt_vals[slot_index]);
> > - else
> > - LogMessageVerbSigSafe(X_WARNING, 0,
> > - "%s: Attempted to copy values from out-of-range "
> > - "slot, touch events may be incorrect.\n",
> > - pInfo->name);
> > - } else
> > - pEvdev->slot_state = SLOTSTATE_CLOSE;
> > + valuator_mask_copy(pEvdev->mt_mask,
> > + pEvdev->last_mt_vals[slot_index]);
> > + } else if (pEvdev->slots[slot_index].state != SLOTSTATE_EMPTY)
> > + pEvdev->slots[slot_index].state = SLOTSTATE_CLOSE;
> > } else {
> > map = pEvdev->abs_axis_map[ev->code];
> > valuator_mask_set(pEvdev->mt_mask, map, ev->value);
> > - if (slot_index >= 0)
> > - valuator_mask_set(pEvdev->last_mt_vals[slot_index], map,
> > - ev->value);
> > + valuator_mask_set(pEvdev->last_mt_vals[slot_index], map,
> > + ev->value);
> > }
> > }
> > }
> > @@ -1041,6 +1049,8 @@ EvdevFreeMasks(EvdevPtr pEvdev)
> > int i;
> > #endif
> >
> > + free(pEvdev->slots);
> > + pEvdev->slots = NULL;
> > valuator_mask_free(&pEvdev->vals);
> > valuator_mask_free(&pEvdev->old_vals);
> > valuator_mask_free(&pEvdev->prox);
> > @@ -1320,6 +1330,17 @@ EvdevAddAbsValuatorClass(DeviceIntPtr device, int want_scroll_axes)
> > goto out;
> > }
> >
> > + pEvdev->slots = calloc(nslots, sizeof(*pEvdev->slots));
> > + if (!pEvdev->slots) {
> > + xf86Msg(X_ERROR, "%s: failed to allocate slot state array.\n",
> > + device->name);
> > + goto out;
> > + }
> > + for (i = 0; i < nslots; i++) {
> > + pEvdev->slots[i].state = SLOTSTATE_EMPTY;
> > + pEvdev->slots[i].dirty = 0;
> > + }
> > +
> > pEvdev->last_mt_vals = calloc(nslots, sizeof(ValuatorMask *));
> > if (!pEvdev->last_mt_vals) {
> > xf86IDrvMsg(pInfo, X_ERROR,
> > diff --git a/src/evdev.h b/src/evdev.h
> > index 520d017..2d6b62d 100644
> > --- a/src/evdev.h
> > +++ b/src/evdev.h
> > @@ -167,7 +167,10 @@ typedef struct {
> > ValuatorMask *mt_mask;
> > ValuatorMask **last_mt_vals;
> > int cur_slot;
> > - enum SlotState slot_state;
> > + struct slot {
> > + int dirty;
> > + enum SlotState state;
> > + } *slots;
> > #ifdef MULTITOUCH
> > struct mtdev *mtdev;
> > #endif
More information about the xorg-devel
mailing list