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 <[email protected]> > 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 <[email protected]> > > --- > > 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 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
