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.hutte...@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 */ _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel