[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