[PATCH xf86-input-evdev 2/2 v2] Add support for masked valuators
Peter Hutterer
peter.hutterer at who-t.net
Wed Jan 5 19:33:38 PST 2011
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 <chase.douglas at canonical.com>
> ---
> 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
>
More information about the xorg-devel
mailing list