[PATCH] input: fix mouse wheel support for DGA clients
Peter Hutterer
peter.hutterer at who-t.net
Tue May 22 23:04:33 PDT 2012
On Mon, May 21, 2012 at 09:39:43PM +0200, Marcin Slusarz wrote:
> On Mon, May 21, 2012 at 05:05:45PM +1000, Peter Hutterer wrote:
> > On Mon, May 21, 2012 at 01:16:40AM +0200, Marcin Slusarz wrote:
> > > xf86-input-evdev (since "smooth scrolling" support was added) can send mouse
> > > motion and wheel events in one batch, so we need to handle it properly.
> > > Otherwise mouse wheel events which came with motion events are lost
> > > and separate mouse wheel events are handled through non-DGA path.
> > >
> > > Signed-off-by: Marcin Slusarz <marcin.slusarz at gmail.com>
> > > ---
> > > hw/xfree86/common/xf86Xinput.c | 77 ++++++++++++++++++++++++++++++++--------
> > > 1 files changed, 62 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
> > > index d246d2a..bf615ad 100644
> > > --- a/hw/xfree86/common/xf86Xinput.c
> > > +++ b/hw/xfree86/common/xf86Xinput.c
> > > @@ -1059,24 +1059,16 @@ xf86PostMotionEventP(DeviceIntPtr device,
> > > xf86PostMotionEventM(device, is_absolute, &mask);
> > > }
> > >
> > > -void
> > > -xf86PostMotionEventM(DeviceIntPtr device,
> > > - int is_absolute, const ValuatorMask *mask)
> > > +#if XFreeXDGA
> > > +static int xf86CheckMotionEvent4DGA(DeviceIntPtr device, int is_absolute,
> > > + const ValuatorMask *mask)
> > > {
> >
> > put the #if inside here (afer stolen = 0), so we don't have two declarations
> > of the same function.
>
> Ok, kernel style undone.
>
> > (also, general note, we require 4-space indentation now, no tabs)
>
> fixed
>
> > > - int flags = 0;
> > > -
> > > - if (valuator_mask_num_valuators(mask) > 0) {
> > > - if (is_absolute)
> > > - flags = POINTER_ABSOLUTE;
> > > - else
> > > - flags = POINTER_RELATIVE | POINTER_ACCELERATE;
> > > - }
> > > + int stolen = 0;
> > >
> > > -#if XFreeXDGA
> > > /* The evdev driver may not always send all axes across. */
> > > if (valuator_mask_isset(mask, 0) || valuator_mask_isset(mask, 1))
> > > if (miPointerGetScreen(device)) {
> > > - int index = miPointerGetScreen(device)->myNum;
> > > + int idx = miPointerGetScreen(device)->myNum;
> >
> > any particular reason you changed this name?
>
> "index" is libc function, so gcc generates:
>
> warning: declaration of ‘index’ shadows a global declaration
>
> > > int dx = 0, dy = 0;
> > >
> > > if (valuator_mask_isset(mask, 0)) {
> > > @@ -1091,11 +1083,66 @@ xf86PostMotionEventM(DeviceIntPtr device,
> > > dy -= device->last.valuators[1];
> > > }
> > >
> > > - if (DGAStealMotionEvent(device, index, dx, dy))
> > > - return;
> > > + if (DGAStealMotionEvent(device, idx, dx, dy))
> > > + stolen = 1;
> > > + }
> > > +
> > > + if (valuator_mask_isset(mask, 2))
> >
> > please wrap this in {} as well
>
> ok
>
> > > + if (miPointerGetScreen(device)) {
> > > + int idx = miPointerGetScreen(device)->myNum;
> > > +
> > > + if (valuator_mask_get(mask, 2) > 0) {
> > > + if (DGAStealButtonEvent(device, idx, 4, 1) &&
> > > + DGAStealButtonEvent(device, idx, 4, 0))
> > > + stolen = 1;
> > > + } else {
> > > + if (DGAStealButtonEvent(device, idx, 5, 1) &&
> > > + DGAStealButtonEvent(device, idx, 5, 0))
> > > + stolen = 1;
> > > + }
> > > + }
> > > +
> > > + if (valuator_mask_isset(mask, 3))
> > > + if (miPointerGetScreen(device)) {
> > > + int idx = miPointerGetScreen(device)->myNum;
> > > +
> > > + if (valuator_mask_get(mask, 3) > 0) {
> > > + if (DGAStealButtonEvent(device, idx, 7, 1) &&
> > > + DGAStealButtonEvent(device, idx, 7, 0))
> > > + stolen = 1;
> > > + } else {
> > > + if (DGAStealButtonEvent(device, idx, 6, 1) &&
> > > + DGAStealButtonEvent(device, idx, 6, 0))
> > > + stolen = 1;
> > > + }
> > > }
> >
> > not quite correct, we can't guarantee that valuators 2/3 are the scrolling
> > axes on every device. you need to check if the
> > valuator->axes[axis].scroll.type is something other than SCROLL_TYPE_NONE.
> > emulate_scroll_button_events in dix/getevents.c is the blueprint here,
> > should be easy enough to follow (or possibly re-use)
>
> Thank you. Updated patch below.
>
> BTW, this patch probably fixes https://bugs.launchpad.net/ubuntu/+source/xorg-server/+bug/953960.
> Initially, I also thought this bug appears in combination with keyboard action.
>
> ---
> From: Marcin Slusarz <marcin.slusarz at gmail.com>
> Subject: [PATCH v2] input: fix mouse wheel support for DGA clients
>
> xf86-input-evdev (since "smooth scrolling" support was added) can send mouse
> motion and wheel events in one batch, so we need to handle it properly.
> Otherwise mouse wheel events which come with motion events are lost
> and separate mouse wheel events are handled through non-DGA path.
>
> Signed-off-by: Marcin Slusarz <marcin.slusarz at gmail.com>
> ---
> HWheel support was not tested (no hardware;)
> ---
> hw/xfree86/common/xf86Xinput.c | 93 +++++++++++++++++++++++++++++++++-------
> 1 files changed, 77 insertions(+), 16 deletions(-)
>
> diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
> index d246d2a..d63a99e 100644
> --- a/hw/xfree86/common/xf86Xinput.c
> +++ b/hw/xfree86/common/xf86Xinput.c
> @@ -1059,26 +1059,23 @@ xf86PostMotionEventP(DeviceIntPtr device,
> xf86PostMotionEventM(device, is_absolute, &mask);
> }
>
> -void
> -xf86PostMotionEventM(DeviceIntPtr device,
> - int is_absolute, const ValuatorMask *mask)
> +static int xf86CheckMotionEvent4DGA(DeviceIntPtr device, int is_absolute,
> + const ValuatorMask *mask)
> {
merged thanks.
two changes: this declaration broken to static int on it's own line, commit
msg changed from "input: ..." to "xfree86: ..."
Cheers,
Peter
> - int flags = 0;
> -
> - if (valuator_mask_num_valuators(mask) > 0) {
> - if (is_absolute)
> - flags = POINTER_ABSOLUTE;
> - else
> - flags = POINTER_RELATIVE | POINTER_ACCELERATE;
> - }
> + int stolen = 0;
>
> #if XFreeXDGA
> + ScreenPtr scr = NULL;
> + int idx, i;
> +
> /* The evdev driver may not always send all axes across. */
> - if (valuator_mask_isset(mask, 0) || valuator_mask_isset(mask, 1))
> - if (miPointerGetScreen(device)) {
> - int index = miPointerGetScreen(device)->myNum;
> + if (valuator_mask_isset(mask, 0) || valuator_mask_isset(mask, 1)) {
> + scr = miPointerGetScreen(device);
> + if (scr) {
> int dx = 0, dy = 0;
>
> + idx = scr->myNum;
> +
> if (valuator_mask_isset(mask, 0)) {
> dx = valuator_mask_get(mask, 0);
> if (is_absolute)
> @@ -1091,11 +1088,75 @@ xf86PostMotionEventM(DeviceIntPtr device,
> dy -= device->last.valuators[1];
> }
>
> - if (DGAStealMotionEvent(device, index, dx, dy))
> - return;
> + if (DGAStealMotionEvent(device, idx, dx, dy))
> + stolen = 1;
> }
> + }
> +
> + for (i = 2; i < valuator_mask_size(mask); i++) {
> + AxisInfoPtr ax;
> + double incr;
> + int val, button;
> +
> + if (i >= device->valuator->numAxes)
> + break;
> +
> + if (!valuator_mask_isset(mask, i))
> + continue;
> +
> + ax = &device->valuator->axes[i];
> +
> + if (ax->scroll.type == SCROLL_TYPE_NONE)
> + continue;
> +
> + if (!scr) {
> + scr = miPointerGetScreen(device);
> + if (!scr)
> + break;
> + idx = scr->myNum;
> + }
> +
> + incr = ax->scroll.increment;
> + val = valuator_mask_get(mask, i);
> +
> + if (ax->scroll.type == SCROLL_TYPE_VERTICAL) {
> + if (incr * val < 0)
> + button = 4; /* up */
> + else
> + button = 5; /* down */
> + } else { /* SCROLL_TYPE_HORIZONTAL */
> + if (incr * val < 0)
> + button = 6; /* left */
> + else
> + button = 7; /* right */
> + }
> +
> + if (DGAStealButtonEvent(device, idx, button, 1) &&
> + DGAStealButtonEvent(device, idx, button, 0))
> + stolen = 1;
> + }
> +
> #endif
>
> + return stolen;
> +}
> +
> +void
> +xf86PostMotionEventM(DeviceIntPtr device,
> + int is_absolute, const ValuatorMask *mask)
> +{
> + int flags = 0;
> +
> + if (xf86CheckMotionEvent4DGA(device, is_absolute, mask))
> + return;
> +
> + if (valuator_mask_num_valuators(mask) > 0) {
> + if (is_absolute)
> + flags = POINTER_ABSOLUTE;
> + else
> + flags = POINTER_RELATIVE | POINTER_ACCELERATE;
> + }
> +
> QueuePointerEvents(device, MotionNotify, 0, flags, mask);
> }
>
> --
> 1.7.8.6
>
More information about the xorg-devel
mailing list