[PATCH evdev 3/3] Allow relative scroll valuators on absolute devices
Chase Douglas
chase.douglas at canonical.com
Wed Mar 28 20:19:00 PDT 2012
On 03/28/2012 05:09 PM, Peter Hutterer wrote:
> Special-case REL_WHEEL, REL_HWHEEL and REL_DIAL to add scroll valuators
> for those axes on top of the absolute axes.
> REL_* and ABS_* overlap, so we need to offset the REL_* axes to avoid
> overwriting a potential ABS_* mapping. REL_* axes mappings are stored after
> the last absolute axis detected, for purely relative devices that offset is
> simply 0.
>
> This isn't needed before smooth-scrolling support as REL_WHEEL and friends
> are special-cased in EvdevProcessRelativeMotionEvent, north of the first
> hunk in this patch.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=805902
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
I think over time we've painted ourselves into a corner with axes in
evdev. This patch screams *HACK* *HACK* *HACK*, mostly because of having
one axis array and using a marker for switching between absolute and
relative axes.
* Would it be possible to use separate axis maps for relative and
absolute axes?
* Are we going to pay later for having variables like "want_scroll_axes"
that configure axes like REL_DIAL? The variable is essentially
controlling a whitelist. Would we be better off having a default
blacklist to disable REL_X and REL_Y if ABS_X and ABS_Y are supported,
and letting everything else through?
> ---
> src/evdev.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
> src/evdev.h | 3 +-
> 2 files changed, 99 insertions(+), 6 deletions(-)
>
> diff --git a/src/evdev.c b/src/evdev.c
> index 7848c9d..91e314d 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -864,7 +864,9 @@ EvdevProcessRelativeMotionEvent(InputInfoPtr pInfo, struct input_event *ev)
> #endif
> default:
> /* Ignore EV_REL events if we never set up for them. */
> - if (!(pEvdev->flags & EVDEV_RELATIVE_EVENTS))
> + if (!(pEvdev->flags & EVDEV_RELATIVE_EVENTS) &&
> + ev->code != REL_WHEEL && ev->code != REL_DIAL &&
> + ev->code != REL_HWHEEL)
> return;
>
> /* Handle mouse wheel emulation */
> @@ -873,7 +875,7 @@ EvdevProcessRelativeMotionEvent(InputInfoPtr pInfo, struct input_event *ev)
>
> pEvdev->rel_queued = 1;
> pEvdev->delta[ev->code] += value;
> - map = pEvdev->axis_map[ev->code];
> + map = pEvdev->axis_map[ev->code + pEvdev->rel_offset];
> valuator_mask_set(pEvdev->vals, map, value);
> break;
> }
> @@ -1396,7 +1398,7 @@ is_blacklisted_axis(int axis)
>
>
> static int
> -EvdevAddAbsValuatorClass(DeviceIntPtr device)
> +EvdevAddAbsValuatorClass(DeviceIntPtr device, int want_scroll_axes)
> {
> InputInfoPtr pInfo;
> EvdevPtr pEvdev;
> @@ -1444,6 +1446,20 @@ EvdevAddAbsValuatorClass(DeviceIntPtr device)
> }
> }
> #endif
> +
> +#ifdef HAVE_SMOOTH_SCROLLING
> + if (want_scroll_axes && EvdevBitIsSet(pEvdev->bitmask, EV_REL))
> + {
> + pEvdev->rel_offset = num_axes;
> + if (EvdevBitIsSet(pEvdev->rel_bitmask, REL_WHEEL))
> + num_axes++;
> + if (EvdevBitIsSet(pEvdev->rel_bitmask, REL_HWHEEL))
> + num_axes++;
> + if (EvdevBitIsSet(pEvdev->rel_bitmask, REL_DIAL))
> + num_axes++;
> + }
> +#endif
> +
> if (num_axes + num_mt_axes > MAX_VALUATORS) {
> xf86IDrvMsg(pInfo, X_WARNING, "found %d axes, limiting to %d.\n", num_axes, MAX_VALUATORS);
> num_axes = MAX_VALUATORS;
> @@ -1532,6 +1548,33 @@ EvdevAddAbsValuatorClass(DeviceIntPtr device)
>
> EvdevInitAxesLabels(pEvdev, Absolute, pEvdev->num_vals + num_mt_axes, atoms);
>
> +#ifdef HAVE_SMOOTH_SCROLLING
> + if (want_scroll_axes)
> + {
> + int mapping = pEvdev->rel_offset;
> + int axis;
> +
> + if (EvdevBitIsSet(pEvdev->rel_bitmask, REL_HWHEEL))
> + {
> + axis = pEvdev->rel_offset + REL_HWHEEL;
> + pEvdev->axis_map[axis] = mapping++;
> + EvdevInitOneAxisLabel(pEvdev, axis, rel_labels, REL_HWHEEL, atoms);
> + }
> + if (EvdevBitIsSet(pEvdev->rel_bitmask, REL_DIAL))
> + {
> + axis = pEvdev->rel_offset + REL_DIAL;
> + pEvdev->axis_map[axis] = mapping++;
> + EvdevInitOneAxisLabel(pEvdev, axis, rel_labels, REL_DIAL, atoms);
> + }
> + if (EvdevBitIsSet(pEvdev->rel_bitmask, REL_WHEEL))
> + {
> + axis = pEvdev->rel_offset + REL_WHEEL;
> + pEvdev->axis_map[axis] = mapping++;
> + EvdevInitOneAxisLabel(pEvdev, axis, rel_labels, REL_WHEEL, atoms);
> + }
> + }
> +#endif
> +
> if (!InitValuatorClassDeviceStruct(device, num_axes + num_mt_axes, atoms,
> GetMotionHistorySize(), Absolute)) {
> xf86IDrvMsg(pInfo, X_ERROR, "failed to initialize valuator class device.\n");
> @@ -1623,6 +1666,51 @@ EvdevAddAbsValuatorClass(DeviceIntPtr device)
> }
> #endif
>
> +#ifdef HAVE_SMOOTH_SCROLLING
> + if (want_scroll_axes)
> + {
> + int axidx;
> + if (EvdevBitIsSet(pEvdev->rel_bitmask, REL_WHEEL))
> + {
> + axidx = pEvdev->rel_offset + REL_WHEEL;
What is the meaning of the variable name "axidx"? I think it's supposed
to mean "axis index", but it was not obvious to me at first glance. I'd
suggest coming up with a different name. "index" might be good enough
since it's obvious we're talking about axes here. "axis" is also used
just above this in the diff without issue.
> + xf86InitValuatorAxisStruct(device,
> + pEvdev->axis_map[axidx],
> + atoms[pEvdev->axis_map[axidx]],
> + NO_AXIS_LIMITS, NO_AXIS_LIMITS,
> + 0, 0, 0, Relative);
> + SetScrollValuator(device, pEvdev->axis_map[axidx],
> + SCROLL_TYPE_VERTICAL, -1.0,
> + SCROLL_FLAG_PREFERRED);
> + }
> +
> + if (EvdevBitIsSet(pEvdev->rel_bitmask, REL_HWHEEL))
> + {
> + axidx = pEvdev->rel_offset + REL_HWHEEL;
> + xf86InitValuatorAxisStruct(device,
> + pEvdev->axis_map[axidx],
> + atoms[pEvdev->axis_map[axidx]],
> + NO_AXIS_LIMITS, NO_AXIS_LIMITS,
> + 0, 0, 0, Relative);
> + SetScrollValuator(device, pEvdev->axis_map[axidx],
> + SCROLL_TYPE_HORIZONTAL, 1.0,
> + SCROLL_FLAG_NONE);
> + }
> +
> + if (EvdevBitIsSet(pEvdev->rel_bitmask, REL_DIAL))
> + {
> + axidx = pEvdev->rel_offset + REL_DIAL;
> + xf86InitValuatorAxisStruct(device,
> + pEvdev->axis_map[axidx],
> + atoms[pEvdev->axis_map[axidx]],
> + NO_AXIS_LIMITS, NO_AXIS_LIMITS,
> + 0, 0, 0, Relative);
> + SetScrollValuator(device, pEvdev->axis_map[axidx],
> + SCROLL_TYPE_HORIZONTAL, 1.0,
> + SCROLL_FLAG_NONE);
> + }
> + }
> +#endif
> +
> free(atoms);
>
> for (i = 0; i < ArrayLength(proximity_bits); i++)
> @@ -1852,12 +1940,16 @@ static void
> EvdevInitAnyValuators(DeviceIntPtr device, EvdevPtr pEvdev)
> {
> InputInfoPtr pInfo = device->public.devicePrivate;
> + int rel_success = FALSE;
>
> if (pEvdev->flags & EVDEV_RELATIVE_EVENTS &&
> EvdevAddRelValuatorClass(device) == Success)
> + {
> + rel_success = TRUE;
The variable "rel_success" doesn't carry much connotation by itself. I
think "has_rel_axes" would be better.
> xf86IDrvMsg(pInfo, X_INFO, "initialized for relative axes.\n");
> + }
> if (pEvdev->flags & EVDEV_ABSOLUTE_EVENTS &&
> - EvdevAddAbsValuatorClass(device) == Success)
> + EvdevAddAbsValuatorClass(device, !rel_success) == Success)
> xf86IDrvMsg(pInfo, X_INFO, "initialized for absolute axes.\n");
> }
>
> @@ -1866,7 +1958,7 @@ EvdevInitAbsValuators(DeviceIntPtr device, EvdevPtr pEvdev)
> {
> InputInfoPtr pInfo = device->public.devicePrivate;
>
> - if (EvdevAddAbsValuatorClass(device) == Success) {
> + if (EvdevAddAbsValuatorClass(device, TRUE) == Success) {
> xf86IDrvMsg(pInfo, X_INFO,"initialized for absolute axes.\n");
> } else {
> xf86IDrvMsg(pInfo, X_ERROR,"failed to initialize for absolute axes.\n");
> diff --git a/src/evdev.h b/src/evdev.h
> index 309b215..cca5cbe 100644
> --- a/src/evdev.h
> +++ b/src/evdev.h
> @@ -153,7 +153,8 @@ typedef struct {
> int grabDevice; /* grab the event device? */
>
> int num_vals; /* number of valuators */
> - int axis_map[max(ABS_CNT, REL_CNT)]; /* Map evdev <axis> to index */
> + int axis_map[ABS_CNT + REL_CNT]; /* Map evdev <axis> to index */
> + int rel_offset; /* offset for relative axes in absolute device */
> ValuatorMask *vals; /* new values coming in */
> ValuatorMask *old_vals; /* old values for calculating relative motion */
> ValuatorMask *prox; /* last values set while not in proximity */
More information about the xorg-devel
mailing list