On Wed, Jan 05, 2011 at 03:27:06PM -0500, Chase Douglas wrote: > With the X server now supporting masked valuators for XI2, enable > support in X evdev. > > This kills backwards compatibility with X Input ABI < 12
thanks! we need some extra configure checks to prevent building against older servers, and once we do that, all current ABI_XINPUT checks can go too. > > Signed-off-by: Chase Douglas <[email protected]> > --- > Performed further testing and found a crash in the proximity code when > using an ALPS (Synaptics-like) touchpad. This version has a simple check > for proximity capability before processing any proximity state. > > src/emuWheel.c | 5 +- > src/evdev.c | 201 > ++++++++++++++++++++++++++++++++------------------------ > src/evdev.h | 6 +- > 3 files changed, 121 insertions(+), 91 deletions(-) > > diff --git a/src/emuWheel.c b/src/emuWheel.c > index 9a53211..c8683d9 100644 > --- a/src/emuWheel.c > +++ b/src/emuWheel.c > @@ -120,8 +120,9 @@ EvdevWheelEmuFilterMotion(InputInfoPtr pInfo, struct > input_event *pEv) > > /* We don't want to intercept real mouse wheel events */ > if(pEv->type == EV_ABS) { > - oldValue = pEvdev->vals[pEvdev->axis_map[pEv->code]]; > - pEvdev->vals[pEvdev->axis_map[pEv->code]] = value; > + int axis = pEvdev->axis_map[pEv->code]; > + oldValue = valuator_mask_get(pEvdev->mask, axis); > + valuator_mask_set(pEvdev->mask, axis, value); > value -= oldValue; /* make value into a differential measurement */ > } > > diff --git a/src/evdev.c b/src/evdev.c > index 45873c1..c8bf381 100644 > --- a/src/evdev.c > +++ b/src/evdev.c > @@ -368,41 +368,41 @@ EvdevQueueButtonClicks(InputInfoPtr pInfo, int button, > int count) > } > } > > -#define ABS_X_VALUE 0x1 > -#define ABS_Y_VALUE 0x2 > -#define ABS_VALUE 0x4 > /** > * Take the valuators and process them accordingly. > */ > static void > -EvdevProcessValuators(InputInfoPtr pInfo, int v[MAX_VALUATORS], int *num_v, > - int *first_v) > +EvdevProcessValuators(InputInfoPtr pInfo) > { > int tmp; > EvdevPtr pEvdev = pInfo->private; > > - *num_v = *first_v = 0; > - > /* convert to relative motion for touchpads */ > if (pEvdev->abs_queued && (pEvdev->flags & EVDEV_RELATIVE_MODE)) { > if (pEvdev->in_proximity) { > - if (pEvdev->old_vals[0] != -1) > - pEvdev->delta[REL_X] = pEvdev->vals[0] - pEvdev->old_vals[0]; > - if (pEvdev->old_vals[1] != -1) > - pEvdev->delta[REL_Y] = pEvdev->vals[1] - pEvdev->old_vals[1]; > - if (pEvdev->abs_queued & ABS_X_VALUE) > - pEvdev->old_vals[0] = pEvdev->vals[0]; > - if (pEvdev->abs_queued & ABS_Y_VALUE) > - pEvdev->old_vals[1] = pEvdev->vals[1]; > + if (valuator_mask_isset(pEvdev->mask, 0) && > + valuator_mask_isset(pEvdev->oldMask, 0)) > + pEvdev->delta[REL_X] = valuator_mask_get(pEvdev->mask, 0) - > + valuator_mask_get(pEvdev->oldMask, 0); > + if (valuator_mask_isset(pEvdev->mask, 1) && > + valuator_mask_isset(pEvdev->oldMask, 1)) > + pEvdev->delta[REL_Y] = valuator_mask_get(pEvdev->mask, 1) - > + valuator_mask_get(pEvdev->oldMask, 1); > + if (valuator_mask_isset(pEvdev->mask, 0)) > + valuator_mask_set(pEvdev->oldMask, 0, > + valuator_mask_get(pEvdev->mask, 0)); > + if (valuator_mask_isset(pEvdev->mask, 1)) > + valuator_mask_set(pEvdev->oldMask, 1, > + valuator_mask_get(pEvdev->mask, 1)); please rearrange this into if (valuator_mask_isset(mask, 0)) { if (valuator_mask_isset(oldmask, 0) delta = ... valuator_mask_set(oldMask) } > } else { > - pEvdev->old_vals[0] = pEvdev->old_vals[1] = -1; > + valuator_mask_zero(pEvdev->oldMask); > } > + valuator_mask_zero(pEvdev->mask); > pEvdev->abs_queued = 0; > pEvdev->rel_queued = 1; > } > > if (pEvdev->rel_queued) { > - int first = REL_CNT, last = -1; > int i; > > if (pEvdev->swap_axes) { > @@ -419,19 +419,7 @@ EvdevProcessValuators(InputInfoPtr pInfo, int > v[MAX_VALUATORS], int *num_v, > { > int map = pEvdev->axis_map[i]; > if (pEvdev->delta[i] && map != -1) > - { > - v[map] = pEvdev->delta[i]; > - if (map < first) > - first = map; > - if (map > last) > - last = map; > - } > - } > - > - if (last >= 0) > - { > - *num_v = (last - first + 1); > - *first_v = first; > + valuator_mask_set(pEvdev->mask, map, pEvdev->delta[i]); > } > } > /* > @@ -444,43 +432,39 @@ EvdevProcessValuators(InputInfoPtr pInfo, int > v[MAX_VALUATORS], int *num_v, > * just works. > */ > else if (pEvdev->abs_queued && pEvdev->in_proximity) { > - memcpy(v, pEvdev->vals, sizeof(int) * pEvdev->num_vals); > + int unswapped_x = valuator_mask_get(pEvdev->mask, 0); > + int unswapped_y = valuator_mask_get(pEvdev->mask, 1); > + int i; > > - if (pEvdev->swap_axes) { > - int tmp = v[0]; > - v[0] = xf86ScaleAxis(v[1], > - pEvdev->absinfo[ABS_X].maximum, > - pEvdev->absinfo[ABS_X].minimum, > - pEvdev->absinfo[ABS_Y].maximum, > - pEvdev->absinfo[ABS_Y].minimum); > - v[1] = xf86ScaleAxis(tmp, > - pEvdev->absinfo[ABS_Y].maximum, > - pEvdev->absinfo[ABS_Y].minimum, > - pEvdev->absinfo[ABS_X].maximum, > - pEvdev->absinfo[ABS_X].minimum); > - } > + for (i = 0; i <= 1; i++) { > + int val; > > - if (pEvdev->flags & EVDEV_CALIBRATED) > - { > - v[0] = xf86ScaleAxis(v[0], > - pEvdev->absinfo[ABS_X].maximum, > - pEvdev->absinfo[ABS_X].minimum, > - pEvdev->calibration.max_x, pEvdev->calibration.min_x); > - v[1] = xf86ScaleAxis(v[1], > - pEvdev->absinfo[ABS_Y].maximum, > - pEvdev->absinfo[ABS_Y].minimum, > - pEvdev->calibration.max_y, pEvdev->calibration.min_y); > - } > + if (!valuator_mask_isset(pEvdev->mask, i)) > + continue; > > - if (pEvdev->invert_x) > - v[0] = (pEvdev->absinfo[ABS_X].maximum - v[0] + > - pEvdev->absinfo[ABS_X].minimum); > - if (pEvdev->invert_y) > - v[1] = (pEvdev->absinfo[ABS_Y].maximum - v[1] + > - pEvdev->absinfo[ABS_Y].minimum); > + val = valuator_mask_get(pEvdev->mask, i); val here has the same value as unswapped_x/y, so you can skip those two and just work with val. > > - *num_v = pEvdev->num_vals; > - *first_v = 0; > + if (pEvdev->swap_axes) > + val = xf86ScaleAxis((i == 0 ? unswapped_y : unswapped_x), > + pEvdev->absinfo[i].maximum, > + pEvdev->absinfo[i].minimum, > + pEvdev->absinfo[1 - i].maximum, > + pEvdev->absinfo[1 - i].minimum); > + > + if (pEvdev->flags & EVDEV_CALIBRATED) > + val = xf86ScaleAxis(val, pEvdev->absinfo[i].maximum, > + pEvdev->absinfo[i].minimum, > + (i == 0 ? pEvdev->calibration.max_x : > + pEvdev->calibration.max_y), > + (i == 0 ? pEvdev->calibration.min_x : > + pEvdev->calibration.min_y)); please, no two-line ternery conditions in function calls. just use a temp variable here. mind you, if you want to get really fancy you can change calibration into a struct { int min; int max; } calibration[2]. but I'm not sure how much that gains us here besides saving one line. > + > + if ((i == 0 && pEvdev->invert_x) || (i == 1 && pEvdev->invert_y)) > + val = (pEvdev->absinfo[i].maximum - val + > + pEvdev->absinfo[i].minimum); > + > + valuator_mask_set(pEvdev->mask, i, val); > + } > } > } > > @@ -518,11 +502,15 @@ EvdevProcessProximityState(InputInfoPtr pInfo) > int prox_state = 0; > int i; > > + /* Does this device have any proximity keys? */ s/keys/axes/ > + if (!pEvdev->proxMask) > + return 0; > + > /* no proximity change in the queue */ > if (!pEvdev->prox_queued) > { > if (pEvdev->abs_queued && !pEvdev->in_proximity) > - pEvdev->abs_prox = pEvdev->abs_queued; > + valuator_mask_copy(pEvdev->proxMask, pEvdev->mask); > return 0; > } > > @@ -540,10 +528,12 @@ EvdevProcessProximityState(InputInfoPtr pInfo) > { > /* We're about to go into/out of proximity but have no abs events > * within the EV_SYN. Use the last coordinates we have. */ > - if (!pEvdev->abs_queued && pEvdev->abs_prox) > + if (!pEvdev->abs_queued && > + valuator_mask_num_valuators(pEvdev->proxMask) > 0) > { > - pEvdev->abs_queued = pEvdev->abs_prox; > - pEvdev->abs_prox = 0; > + valuator_mask_copy(pEvdev->mask, pEvdev->proxMask); > + valuator_mask_zero(pEvdev->proxMask); > + pEvdev->abs_queued = 1; > } > } > > @@ -590,6 +580,7 @@ EvdevProcessRelativeMotionEvent(InputInfoPtr pInfo, > struct input_event *ev) > { > int value; > EvdevPtr pEvdev = pInfo->private; > + int map; > > /* Get the signed value, earlier kernels had this as unsigned */ > value = ev->value; > @@ -622,6 +613,8 @@ EvdevProcessRelativeMotionEvent(InputInfoPtr pInfo, > struct input_event *ev) > > pEvdev->rel_queued = 1; > pEvdev->delta[ev->code] += value; > + map = pEvdev->axis_map[ev->code]; > + valuator_mask_set(pEvdev->mask, map, value); > break; > } > } > @@ -634,6 +627,7 @@ EvdevProcessAbsoluteMotionEvent(InputInfoPtr pInfo, > struct input_event *ev) > { > int value; > EvdevPtr pEvdev = pInfo->private; > + int map; > > /* Get the signed value, earlier kernels had this as unsigned */ > value = ev->value; > @@ -648,13 +642,9 @@ EvdevProcessAbsoluteMotionEvent(InputInfoPtr pInfo, > struct input_event *ev) > if (EvdevWheelEmuFilterMotion(pInfo, ev)) > return; > > - pEvdev->vals[pEvdev->axis_map[ev->code]] = value; > - if (ev->code == ABS_X) > - pEvdev->abs_queued |= ABS_X_VALUE; > - else if (ev->code == ABS_Y) > - pEvdev->abs_queued |= ABS_Y_VALUE; > - else > - pEvdev->abs_queued |= ABS_VALUE; > + map = pEvdev->axis_map[ev->code]; > + valuator_mask_set(pEvdev->mask, map, value); > + pEvdev->abs_queued = 1; > } > > /** > @@ -712,7 +702,7 @@ EvdevPostRelativeMotionEvents(InputInfoPtr pInfo, int > num_v, int first_v, > EvdevPtr pEvdev = pInfo->private; > > if (pEvdev->rel_queued) { > - xf86PostMotionEventP(pInfo->dev, FALSE, first_v, num_v, v + first_v); > + xf86PostMotionEventM(pInfo->dev, FALSE, pEvdev->mask); > } > } > > @@ -735,7 +725,7 @@ EvdevPostAbsoluteMotionEvents(InputInfoPtr pInfo, int > num_v, int first_v, > * this scheme still just work. > */ > if (pEvdev->abs_queued && pEvdev->in_proximity) { > - xf86PostMotionEventP(pInfo->dev, TRUE, first_v, num_v, v + first_v); > + xf86PostMotionEventM(pInfo->dev, TRUE, pEvdev->mask); > } > } > > @@ -806,7 +796,7 @@ EvdevProcessSyncEvent(InputInfoPtr pInfo, struct > input_event *ev) > > EvdevProcessProximityState(pInfo); > > - EvdevProcessValuators(pInfo, v, &num_v, &first_v); > + EvdevProcessValuators(pInfo); > > EvdevPostProximityEvents(pInfo, TRUE, num_v, first_v, v); > EvdevPostRelativeMotionEvents(pInfo, num_v, first_v, v); > @@ -816,6 +806,8 @@ EvdevProcessSyncEvent(InputInfoPtr pInfo, struct > input_event *ev) > > memset(pEvdev->delta, 0, sizeof(pEvdev->delta)); > memset(pEvdev->queue, 0, sizeof(pEvdev->queue)); > + if (pEvdev->mask) > + valuator_mask_zero(pEvdev->mask); > pEvdev->num_queue = 0; > pEvdev->abs_queued = 0; > pEvdev->rel_queued = 0; > @@ -1299,8 +1291,20 @@ EvdevAddAbsClass(DeviceIntPtr device) > } > > pEvdev->num_vals = num_axes; > - memset(pEvdev->vals, 0, num_axes * sizeof(int)); > - memset(pEvdev->old_vals, -1, num_axes * sizeof(int)); > + if (num_axes > 0) { > + pEvdev->mask = valuator_mask_new(num_axes); > + if (!pEvdev->mask) { > + xf86Msg(X_ERROR, "%s: failed to allocate valuator mask.\n", > + device->name); > + return !Success; shouldn't this be goto out as well? > + } > + pEvdev->oldMask = valuator_mask_new(num_axes); > + if (!pEvdev->oldMask) { > + xf86Msg(X_ERROR, "%s: failed to allocate old valuator mask.\n", > + device->name); imo, one error message for both mask and oldMask is enough (mind you, if we fail here we probably topple soon anyway). > + goto out; > + } > + } > atoms = malloc(pEvdev->num_vals * sizeof(Atom)); > > for (axis = ABS_X; i < MAX_VALUATORS && axis <= ABS_MAX; axis++) { > @@ -1323,7 +1327,7 @@ EvdevAddAbsClass(DeviceIntPtr device) > GetMotionHistorySize(), Absolute)) { > xf86Msg(X_ERROR, "%s: failed to initialize valuator class device.\n", > device->name); > - return !Success; > + goto out; > } > > for (axis = ABS_X; axis <= ABS_MAX; axis++) { > @@ -1351,7 +1355,6 @@ EvdevAddAbsClass(DeviceIntPtr device) > #endif > ); > xf86InitValuatorDefaults(device, axnum); > - pEvdev->old_vals[axnum] = -1; > } > > free(atoms); > @@ -1364,6 +1367,12 @@ EvdevAddAbsClass(DeviceIntPtr device) > if (TestBit(proximity_bits[i], pEvdev->key_bitmask)) > { > InitProximityClassDeviceStruct(device); > + pEvdev->proxMask = valuator_mask_new(num_axes); > + if (!pEvdev->proxMask) { > + xf86Msg(X_ERROR, "%s: failed to allocate proximity valuator " > + "mask.\n", device->name); > + goto out; > + } > break; > } > } > @@ -1371,7 +1380,7 @@ EvdevAddAbsClass(DeviceIntPtr device) > if (!InitPtrFeedbackClassDeviceStruct(device, EvdevPtrCtrlProc)) { > xf86Msg(X_ERROR, "%s: failed to initialize pointer feedback class " > "device.\n", device->name); > - return !Success; > + goto out; > } > > if (pEvdev->flags & EVDEV_TOUCHPAD) > @@ -1393,6 +1402,15 @@ EvdevAddAbsClass(DeviceIntPtr device) > } > > return Success; > + > +out: > + free(pEvdev->mask); > + pEvdev->mask = NULL; > + free(pEvdev->oldMask); > + pEvdev->oldMask = NULL; > + free(pEvdev->proxMask); > + pEvdev->proxMask = NULL; > + return !Success; > } > > static int > @@ -1431,7 +1449,11 @@ EvdevAddRelClass(DeviceIntPtr device) > } > > pEvdev->num_vals = num_axes; > - memset(pEvdev->vals, 0, num_axes * sizeof(int)); > + if (num_axes > 0) { > + pEvdev->mask = valuator_mask_new(num_axes); > + if (!pEvdev->mask) > + return !Success; > + } > atoms = malloc(pEvdev->num_vals * sizeof(Atom)); > > for (axis = REL_X; i < MAX_VALUATORS && axis <= REL_MAX; axis++) > @@ -1458,13 +1480,13 @@ EvdevAddRelClass(DeviceIntPtr device) > GetMotionHistorySize(), Relative)) { > xf86Msg(X_ERROR, "%s: failed to initialize valuator class device.\n", > device->name); > - return !Success; > + goto out; > } > > if (!InitPtrFeedbackClassDeviceStruct(device, EvdevPtrCtrlProc)) { > xf86Msg(X_ERROR, "%s: failed to initialize pointer feedback class " > "device.\n", device->name); > - return !Success; > + goto out; > } > > for (axis = REL_X; axis <= REL_MAX; axis++) > @@ -1488,6 +1510,11 @@ EvdevAddRelClass(DeviceIntPtr device) > free(atoms); > > return Success; > + > +out: > + free(pEvdev->mask); > + pEvdev->mask = NULL; > + return !Success; > } please split the "goto out" parts into a separate patch that the valuator mask patch only contains valuator mask portions. > > static int > @@ -1759,6 +1786,9 @@ EvdevProc(DeviceIntPtr device, int what) > close(pInfo->fd); > pInfo->fd = -1; > } > + free(pEvdev->mask); > + free(pEvdev->oldMask); > + free(pEvdev->proxMask); seeing this, I just realised we need a valuator_mask_free() in the server, otherwise the whole point of making the valuator mask opague goes away and we're stuck with a single memory area for the mask. patch is out. > EvdevRemoveDevice(pInfo); > pEvdev->min_maj = 0; > break; > @@ -2015,7 +2045,6 @@ EvdevProbe(InputInfoPtr pInfo) > if (has_lmr || TestBit(BTN_TOOL_FINGER, > pEvdev->key_bitmask)) { > xf86Msg(X_PROBED, "%s: Found absolute touchpad.\n", > pInfo->name); > pEvdev->flags |= EVDEV_TOUCHPAD; > - memset(pEvdev->old_vals, -1, sizeof(int) * > pEvdev->num_vals); > } else { > xf86Msg(X_PROBED, "%s: Found absolute touchscreen\n", > pInfo->name); > pEvdev->flags |= EVDEV_TOUCHSCREEN; > diff --git a/src/evdev.h b/src/evdev.h > index f640fdd..6af145f 100644 > --- a/src/evdev.h > +++ b/src/evdev.h > @@ -121,8 +121,9 @@ typedef struct { > > int num_vals; /* number of valuators */ > int axis_map[max(ABS_CNT, REL_CNT)]; /* Map evdev <axis> to index */ > - int vals[MAX_VALUATORS]; > - int old_vals[MAX_VALUATORS]; /* Translate absolute inputs to relative */ > + ValuatorMask *mask; > + ValuatorMask *oldMask; > + ValuatorMask *proxMask; this is possibly bikeshedding, but why not keep using the names vals and old_vals? the type changes, but it's still used as a valuator array (more or less). please add some comments as to what values the various masks hold, I have to admit the proximity mask confused me a bit. also, evdev is trying to follow a foo_bar naming convention internally where possible. Cheers, Peter > > int flags; > int in_proximity; /* device in proximity */ > @@ -134,7 +135,6 @@ typedef struct { > > int delta[REL_CNT]; > unsigned int abs_queued, rel_queued, prox_queued; > - unsigned int abs_prox; /* valuators posted while out of prox? */ > > /* XKB stuff has to be per-device rather than per-driver */ > #if GET_ABI_MAJOR(ABI_XINPUT_VERSION) < 5 > -- > 1.7.2.3 > _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
