[PATCH] input: fix mouse wheel support for DGA clients

Marcin Slusarz marcin.slusarz at gmail.com
Mon May 21 12:39:43 PDT 2012


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)
 {
-    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