[PATCH 27/27] Input: Add smooth-scrolling support to GetPointerEvents
Peter Hutterer
peter.hutterer at who-t.net
Mon Jun 6 23:29:45 PDT 2011
On Fri, Jun 03, 2011 at 04:00:03PM +0100, Daniel Stone wrote:
> For scroll wheel support, we used to send buttons 4/5 and 6/7 for
> horizontal/vertical positive/negative scroll events. For touchpads, we
> really want more fine-grained scroll values. GetPointerEvents now
> accepts both old-school scroll button presses, and new-style scroll axis
> events, while emitting both types of events to support both old and new
> clients.
>
> Signed-off-by: Daniel Stone <daniel at fooishbar.org>
> ---
> dix/getevents.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 104 insertions(+), 4 deletions(-)
>
> diff --git a/dix/getevents.c b/dix/getevents.c
> index c1ebbb2..f8be73c 100644
> --- a/dix/getevents.c
> +++ b/dix/getevents.c
> @@ -2,6 +2,7 @@
> * Copyright © 2006 Nokia Corporation
> * Copyright © 2006-2007 Daniel Stone
> * Copyright © 2008 Red Hat, Inc.
> + * Copyright © 2011 The Chromium OS Authors
> *
> * Permission is hereby granted, free of charge, to any person obtaining a
> * copy of this software and associated documentation files (the "Software"),
> @@ -60,11 +61,13 @@
> #include <X11/extensions/XI.h>
> #include <X11/extensions/XI2.h>
> #include <X11/extensions/XIproto.h>
> +#include <X11/extensions/XI2.h>
does it scroll smoother if you include it twice? :)
> #include <pixman.h>
> #include "exglobals.h"
> #include "exevents.h"
> #include "extnsionst.h"
> #include "listdev.h" /* for sizing up DeviceClassesChangedEvent */
> +#include "xserver-properties.h"
>
> /* Number of motion history events to store. */
> #define MOTION_HISTORY_SIZE 256
> @@ -622,8 +625,10 @@ GetMaximumEventsNum(void) {
> /* One raw event
> * One device event
> * One possible device changed event
> + * Lots of possible separate scroll events (horiz + vert)
"button scroll events" is less ambiguous now that we have two scrolling
methods.
> + * Lots of possible separate raw scroll event (horiz + vert)
> */
> - return 3;
> + return 51;
> }
>
>
> @@ -1199,7 +1204,12 @@ GetPointerEvents(InternalEvent *events, DeviceIntPtr pDev, int type, int buttons
> int flags, const ValuatorMask *mask_in)
GPE has a rather extensive comment describing what it does. You could
continue this tradition ;)
> {
> CARD32 ms = GetTimeInMillis();
> - int num_events = 0;
> + int num_events = 0, nev_tmp;
> + int i;
> + ValuatorMask mask;
> + Atom h_scroll_label = XIGetKnownProperty(AXIS_LABEL_PROP_REL_HSCROLL);
> + Atom v_scroll_label = XIGetKnownProperty(AXIS_LABEL_PROP_REL_VSCROLL);
> + int h_scroll_axis = -1, v_scroll_axis = -1;
>
> /* refuse events from disabled devices */
> if (!pDev->enabled)
> @@ -1210,8 +1220,98 @@ GetPointerEvents(InternalEvent *events, DeviceIntPtr pDev, int type, int buttons
>
> events = UpdateFromMaster(events, pDev, DEVCHANGE_POINTER_EVENT,
> &num_events);
> - num_events += _GetPointerEvents(events, pDev, type, buttons, ms, flags,
> - mask_in);
> +
> + valuator_mask_copy(&mask, mask_in);
> +
> + /* Find the vertical and horizontal scroll axes, if any. */
> + for (i = 0; i < pDev->valuator->numAxes; i++)
> + {
> + if (h_scroll_label && pDev->valuator->axes[i].label == h_scroll_label)
> + h_scroll_axis = i;
> + else if (v_scroll_label &&
> + pDev->valuator->axes[i].label == v_scroll_label)
> + v_scroll_axis = i;
> + }
this may be better cached in the DeviceIntRec instead of recalculated on
every single pointer event.
> +
> + /* Turn a scroll button press into a smooth-scrolling event. */
> + if (type == ButtonPress &&
> + ((v_scroll_axis != -1 && (buttons == 4 || buttons == 5)) ||
> + (h_scroll_axis != -1 && (buttons == 6 || buttons == 7))))
> + {
> + double val;
> +
> + type = MotionNotify;
> + buttons = 0;
> + if (buttons == 4)
how about switch(buttons) {...}
> + {
> + val = valuator_mask_get_double(&mask, v_scroll_axis) + 1.0;
> + valuator_mask_set_double(&mask, v_scroll_axis, val);
> + }
> + else if (buttons == 5)
> + {
> + val = valuator_mask_get_double(&mask, v_scroll_axis) - 1.0;
> + valuator_mask_set_double(&mask, v_scroll_axis, val);
> + }
> + else if (buttons == 6)
> + {
> + val = valuator_mask_get_double(&mask, v_scroll_axis) + 1.0;
> + valuator_mask_set_double(&mask, h_scroll_axis, val);
> + }
> + else if (buttons == 7)
> + {
> + val = valuator_mask_get_double(&mask, v_scroll_axis) - 1.0;
> + valuator_mask_set_double(&mask, h_scroll_axis, val);
> + }
> + }
> +
> + /* Return the original event set, plus potential legacy scrolling events. */
> + nev_tmp = _GetPointerEvents(events, pDev, type, buttons, ms, flags,
> + &mask);
> + events += nev_tmp;
> + num_events += nev_tmp;
> +
> + while (((v_scroll_axis != -1 &&
> + fabs(pDev->last.valuators[v_scroll_axis]) >= 1.0) ||
> + (h_scroll_axis != -1 &&
> + fabs(pDev->last.valuators[h_scroll_axis]) >= 1.0)) &&
> + (num_events + 4) < GetMaximumEventsNum())
the + 4 requires a comment
> + {
> + int b = 0;
> +
> + if (v_scroll_axis != -1 && pDev->last.valuators[v_scroll_axis] <= -1.0)
> + {
> + pDev->last.valuators[v_scroll_axis] += 1.0;
> + b = 4;
> + }
> + else if (v_scroll_axis != -1 &&
> + pDev->last.valuators[v_scroll_axis] >= 1.0)
> + {
> + pDev->last.valuators[v_scroll_axis] -= 1.0;
> + b = 5;
> + }
> + else if (h_scroll_axis != -1 &&
> + pDev->last.valuators[h_scroll_axis] <= -1.0)
> + {
> + pDev->last.valuators[h_scroll_axis] += 1.0;
> + b = 6;
> + }
> + else if (h_scroll_axis != -1 &&
> + pDev->last.valuators[h_scroll_axis] >= 1.0)
> + {
> + pDev->last.valuators[h_scroll_axis] -= 1.0;
> + b = 7;
> + }
I'd prefer it nested as
if (v_scroll_axis != 1)
{
if (last.valuators < -1)
else if (...)
} else if (h_scroll_axis != 1)
{
}
Also, I'd rather you move this whole while block into a new function.
> +
> + nev_tmp = _GetPointerEvents(events, pDev, ButtonPress, b, ms,
> + POINTER_EMULATED, NULL);
> + events += nev_tmp;
> + num_events += nev_tmp;
> + nev_tmp = _GetPointerEvents(events, pDev, ButtonRelease, b, ms,
> + POINTER_EMULATED, NULL);
> + events += nev_tmp;
> + num_events += nev_tmp;
> + }
> +
> return num_events;
> }
>
> --
> 1.7.5.3
a bit of a side-issue with this approach is that the scroll events lose
valuator information. we hit that one with the wacom driver where one of my
patches started submitting button events with first/num_valuators as 0/0. it
is then impossible for the client to get valuator information without
tracking motion events (which may be quite costly and I'm not sure all
clients do).
so we went back to submitting the current valuator state with _every_ button
event. This patch suffers from the same issue. If you scroll, there is no
information about the other axes. Which for relative devices doesn't matter
much but for absolute devices it certainly does.
Cheers,
Peter
More information about the xorg-devel
mailing list