Stairstep mouse motion

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

Stairstep mouse motion

Henri Kemppainen-2
So I needed to see my thoughts on paper but my desk was so full of stuff
I couldn't make room for pen and paper.  Instead I fired up Gimp, and
drawing with the mouse worked fine until I realized it's next to impossible
to draw diagonal lines that look like lines.

Instead of straight lines, I got waves.  The faster I draw the mouse, the
deeper the waves.  It looked like diagonal mouse motion generated a pair
of pointer motion events, one for the X axis and another for Y.  And Gimp
smoothed that stairstep motion, resulting in waves.  Xev(1) confirmed my
hypothesis.

It turns out wsmouse(4) is breaking the mouse input into multiple events.
This isn't necessarily a bug, since it allows for a very small and simple
event structure which works without modification as new properties (such
as button states, axes, etc.) are added.

Now wsmouse generates all the events it can in a loop before waking up
the process that waits for these events.  So on the receiving side (i.e.
in the xenocara ws(4) driver) we can sum all the consecutive X and Y
deltas from a single read() before issuing a pointer motion event.  This
eliminates the stairsteps as long as the events generated by wsmouse fit
in the buffers involved.

Other approaches would be either extending the event structure, or perhaps
adding a new event type that somehow communicates the intended grouping
for events that follow/precede (but ugh...).  I felt the ws(4) fix was
the least intrusive, and it appears to work just fine here.  What do you
think?

This image (drawn in Gimp) illustrates the problem.  The last two lines
were drawn with my diff applied.

        http://guu.fi/mousebug.png

Index: xenocara/driver/xf86-input-ws/src/ws.c
===================================================================
RCS file: /cvs/xenocara/driver/xf86-input-ws/src/ws.c,v
retrieving revision 1.57
diff -u -p -r1.57 ws.c
--- xenocara/driver/xf86-input-ws/src/ws.c 8 Jul 2012 14:22:03 -0000 1.57
+++ xenocara/driver/xf86-input-ws/src/ws.c 7 Jul 2013 18:33:57 -0000
@@ -474,7 +474,7 @@ wsReadInput(InputInfoPtr pInfo)
 {
  WSDevicePtr priv = (WSDevicePtr)pInfo->private;
  static struct wscons_event eventList[NUMEVENTS];
- int n, c;
+ int n, c, dx, dy;
  struct wscons_event *event = eventList;
  unsigned char *pBuf;
 
@@ -488,10 +488,11 @@ wsReadInput(InputInfoPtr pInfo)
  if (n == 0)
  return;
 
+ dx = dy = 0;
  n /= sizeof(struct wscons_event);
  while (n--) {
  int buttons = priv->lastButtons;
- int dx = 0, dy = 0, dz = 0, dw = 0, ax = 0, ay = 0;
+ int dz = 0, dw = 0, ax = 0, ay = 0;
  int zbutton = 0, wbutton = 0;
 
  switch (event->type) {
@@ -506,11 +507,11 @@ wsReadInput(InputInfoPtr pInfo)
     buttons));
  break;
  case WSCONS_EVENT_MOUSE_DELTA_X:
- dx = event->value;
+ dx += event->value;
  DBG(4, ErrorF("Relative X %d\n", event->value));
  break;
  case WSCONS_EVENT_MOUSE_DELTA_Y:
- dy = -event->value;
+ dy -= event->value;
  DBG(4, ErrorF("Relative Y %d\n", event->value));
  break;
  case WSCONS_EVENT_MOUSE_ABSOLUTE_X:
@@ -548,14 +549,18 @@ wsReadInput(InputInfoPtr pInfo)
  }
  ++event;
 
- if (dx || dy) {
- if (wsWheelEmuFilterMotion(pInfo, dx, dy))
+ if ((dx || dy) && event->type != WSCONS_EVENT_MOUSE_DELTA_X &&
+    event->type != WSCONS_EVENT_MOUSE_DELTA_Y) {
+ int tmpx = dx, tmpy = dy;
+ dx = dy = 0;
+
+ if (wsWheelEmuFilterMotion(pInfo, tmpx, tmpy))
  continue;
 
  /* relative motion event */
  DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
-    dx, dy));
- xf86PostMotionEvent(pInfo->dev, 0, 0, 2, dx, dy);
+    tmpx, tmpy));
+ xf86PostMotionEvent(pInfo->dev, 0, 0, 2, tmpx, tmpy);
  }
  if (dz && priv->Z.negative != WS_NOMAP
     && priv->Z.positive != WS_NOMAP) {
@@ -583,9 +588,9 @@ wsReadInput(InputInfoPtr pInfo)
  ay = tmp;
  }
  if (ax) {
- dx = ax - priv->old_ax;
+ int xdelta = ax - priv->old_ax;
  priv->old_ax = ax;
- if (wsWheelEmuFilterMotion(pInfo, dx, 0))
+ if (wsWheelEmuFilterMotion(pInfo, xdelta, 0))
  continue;
 
  /* absolute position event */
@@ -593,15 +598,24 @@ wsReadInput(InputInfoPtr pInfo)
  xf86PostMotionEvent(pInfo->dev, 1, 0, 1, ax);
  }
  if (ay) {
- dy = ay - priv->old_ay;
+ int ydelta = ay - priv->old_ay;
  priv->old_ay = ay;
- if (wsWheelEmuFilterMotion(pInfo, 0, dy))
+ if (wsWheelEmuFilterMotion(pInfo, 0, ydelta))
  continue;
 
  /* absolute position event */
  DBG(3, ErrorF("postMotionEvent y %d\n", ay));
  xf86PostMotionEvent(pInfo->dev, 1, 1, 1, ay);
  }
+ }
+ if (dx || dy) {
+ if (wsWheelEmuFilterMotion(pInfo, dx, dy))
+ return;
+
+ /* relative motion event */
+ DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
+    dx, dy));
+ xf86PostMotionEvent(pInfo->dev, 0, 0, 2, dx, dy);
  }
  return;
 }



Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

patrick keshishian
On Sun, Jul 7, 2013 at 12:22 PM, Henri Kemppainen <[hidden email]> wrote:
> So I needed to see my thoughts on paper but my desk was so full of stuff
> I couldn't make room for pen and paper.  Instead I fired up Gimp, and
> drawing with the mouse worked fine until I realized it's next to impossible
> to draw diagonal lines that look like lines.
>
> Instead of straight lines, I got waves.  The faster I draw the mouse, the
> deeper the waves.

That is a very cool effect (referring to your image). Having not seen
this issue, I tried drawing a few diagonal lines; this was using the
synaptics touchpad. The lines were pretty smooth looking. Then I
connected a USB mouse and reattempted drawing the diagonal lines.
That's when I see the stair-step effect; still not as wave-like as
yours are.

Just adding a data-point.

--patrick


> It looked like diagonal mouse motion generated a pair
> of pointer motion events, one for the X axis and another for Y.  And Gimp
> smoothed that stairstep motion, resulting in waves.  Xev(1) confirmed my
> hypothesis.
>
> It turns out wsmouse(4) is breaking the mouse input into multiple events.
> This isn't necessarily a bug, since it allows for a very small and simple
> event structure which works without modification as new properties (such
> as button states, axes, etc.) are added.
>
> Now wsmouse generates all the events it can in a loop before waking up
> the process that waits for these events.  So on the receiving side (i.e.
> in the xenocara ws(4) driver) we can sum all the consecutive X and Y
> deltas from a single read() before issuing a pointer motion event.  This
> eliminates the stairsteps as long as the events generated by wsmouse fit
> in the buffers involved.
>
> Other approaches would be either extending the event structure, or perhaps
> adding a new event type that somehow communicates the intended grouping
> for events that follow/precede (but ugh...).  I felt the ws(4) fix was
> the least intrusive, and it appears to work just fine here.  What do you
> think?
>
> This image (drawn in Gimp) illustrates the problem.  The last two lines
> were drawn with my diff applied.
>
>         http://guu.fi/mousebug.png
>
> Index: xenocara/driver/xf86-input-ws/src/ws.c
> ===================================================================
> RCS file: /cvs/xenocara/driver/xf86-input-ws/src/ws.c,v
> retrieving revision 1.57
> diff -u -p -r1.57 ws.c
> --- xenocara/driver/xf86-input-ws/src/ws.c      8 Jul 2012 14:22:03 -0000       1.57
> +++ xenocara/driver/xf86-input-ws/src/ws.c      7 Jul 2013 18:33:57 -0000
> @@ -474,7 +474,7 @@ wsReadInput(InputInfoPtr pInfo)
>  {
>         WSDevicePtr priv = (WSDevicePtr)pInfo->private;
>         static struct wscons_event eventList[NUMEVENTS];
> -       int n, c;
> +       int n, c, dx, dy;
>         struct wscons_event *event = eventList;
>         unsigned char *pBuf;
>
> @@ -488,10 +488,11 @@ wsReadInput(InputInfoPtr pInfo)
>         if (n == 0)
>                 return;
>
> +       dx = dy = 0;
>         n /= sizeof(struct wscons_event);
>         while (n--) {
>                 int buttons = priv->lastButtons;
> -               int dx = 0, dy = 0, dz = 0, dw = 0, ax = 0, ay = 0;
> +               int dz = 0, dw = 0, ax = 0, ay = 0;
>                 int zbutton = 0, wbutton = 0;
>
>                 switch (event->type) {
> @@ -506,11 +507,11 @@ wsReadInput(InputInfoPtr pInfo)
>                             buttons));
>                         break;
>                 case WSCONS_EVENT_MOUSE_DELTA_X:
> -                       dx = event->value;
> +                       dx += event->value;
>                         DBG(4, ErrorF("Relative X %d\n", event->value));
>                         break;
>                 case WSCONS_EVENT_MOUSE_DELTA_Y:
> -                       dy = -event->value;
> +                       dy -= event->value;
>                         DBG(4, ErrorF("Relative Y %d\n", event->value));
>                         break;
>                 case WSCONS_EVENT_MOUSE_ABSOLUTE_X:
> @@ -548,14 +549,18 @@ wsReadInput(InputInfoPtr pInfo)
>                 }
>                 ++event;
>
> -               if (dx || dy) {
> -                       if (wsWheelEmuFilterMotion(pInfo, dx, dy))
> +               if ((dx || dy) && event->type != WSCONS_EVENT_MOUSE_DELTA_X &&
> +                   event->type != WSCONS_EVENT_MOUSE_DELTA_Y) {
> +                       int tmpx = dx, tmpy = dy;
> +                       dx = dy = 0;
> +
> +                       if (wsWheelEmuFilterMotion(pInfo, tmpx, tmpy))
>                                 continue;
>
>                         /* relative motion event */
>                         DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
> -                           dx, dy));
> -                       xf86PostMotionEvent(pInfo->dev, 0, 0, 2, dx, dy);
> +                           tmpx, tmpy));
> +                       xf86PostMotionEvent(pInfo->dev, 0, 0, 2, tmpx, tmpy);
>                 }
>                 if (dz && priv->Z.negative != WS_NOMAP
>                     && priv->Z.positive != WS_NOMAP) {
> @@ -583,9 +588,9 @@ wsReadInput(InputInfoPtr pInfo)
>                         ay = tmp;
>                 }
>                 if (ax) {
> -                       dx = ax - priv->old_ax;
> +                       int xdelta = ax - priv->old_ax;
>                         priv->old_ax = ax;
> -                       if (wsWheelEmuFilterMotion(pInfo, dx, 0))
> +                       if (wsWheelEmuFilterMotion(pInfo, xdelta, 0))
>                                 continue;
>
>                         /* absolute position event */
> @@ -593,15 +598,24 @@ wsReadInput(InputInfoPtr pInfo)
>                         xf86PostMotionEvent(pInfo->dev, 1, 0, 1, ax);
>                 }
>                 if (ay) {
> -                       dy = ay - priv->old_ay;
> +                       int ydelta = ay - priv->old_ay;
>                         priv->old_ay = ay;
> -                       if (wsWheelEmuFilterMotion(pInfo, 0, dy))
> +                       if (wsWheelEmuFilterMotion(pInfo, 0, ydelta))
>                                 continue;
>
>                         /* absolute position event */
>                         DBG(3, ErrorF("postMotionEvent y %d\n", ay));
>                         xf86PostMotionEvent(pInfo->dev, 1, 1, 1, ay);
>                 }
> +       }
> +       if (dx || dy) {
> +               if (wsWheelEmuFilterMotion(pInfo, dx, dy))
> +                       return;
> +
> +               /* relative motion event */
> +               DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
> +                   dx, dy));
> +               xf86PostMotionEvent(pInfo->dev, 0, 0, 2, dx, dy);
>         }
>         return;
>  }
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

Matthieu Herrb
In reply to this post by Henri Kemppainen-2
On Sun, Jul 07, 2013 at 10:22:23PM +0300, Henri Kemppainen wrote:

> So I needed to see my thoughts on paper but my desk was so full of stuff
> I couldn't make room for pen and paper.  Instead I fired up Gimp, and
> drawing with the mouse worked fine until I realized it's next to impossible
> to draw diagonal lines that look like lines.
>
> Instead of straight lines, I got waves.  The faster I draw the mouse, the
> deeper the waves.  It looked like diagonal mouse motion generated a pair
> of pointer motion events, one for the X axis and another for Y.  And Gimp
> smoothed that stairstep motion, resulting in waves.  Xev(1) confirmed my
> hypothesis.
>
> It turns out wsmouse(4) is breaking the mouse input into multiple events.
> This isn't necessarily a bug, since it allows for a very small and simple
> event structure which works without modification as new properties (such
> as button states, axes, etc.) are added.
>
> Now wsmouse generates all the events it can in a loop before waking up
> the process that waits for these events.  So on the receiving side (i.e.
> in the xenocara ws(4) driver) we can sum all the consecutive X and Y
> deltas from a single read() before issuing a pointer motion event.  This
> eliminates the stairsteps as long as the events generated by wsmouse fit
> in the buffers involved.
>
> Other approaches would be either extending the event structure, or perhaps
> adding a new event type that somehow communicates the intended grouping
> for events that follow/precede (but ugh...).  I felt the ws(4) fix was
> the least intrusive, and it appears to work just fine here.  What do you
> think?
>
> This image (drawn in Gimp) illustrates the problem.  The last two lines
> were drawn with my diff applied.
>
> http://guu.fi/mousebug.png

Thanks for the report and patch.

The issue that input drivers devices need high refresh frequency to be
able to achieve high-precision freehand drawing is quite well known¹.

I re-did a few experiments with and without your patch on my laptop
(with the Lenovo track point wihch is a relative device). I could not
reproduce the waves you're getting in Gimp, but without your patch
diagonals show indeed larger staircases effects. Under xfig(1) or
bitmap(1) the difference is much less noticeable. Clearly the
strategy used by applications to trac mouse motion for freehand
drawing are not the same.

I do fear that with some devices your patch will collapse too many
events and make it harder to follow small radius curves.

OTOH, I've always considered that sending many small steps events to
the application, and letting it merge them would generally be better,
even if they come with a too high latency. Having associated
kernel timestamps would help estimating the speed and get better
dynamic interpolation, but the X input driver model doesn't really
make that possible.

So I'd like to see a few more reports of whether that patch helps or
not.

-----
¹) For years X has tried to solve this issue by using asynchrous IO (ie a
signal handler for SIGIO) to handle input events as soon as
possible. Unfortunatly even though it sort-of worked, it let way too
much code run in the signal handler itself, and without major
architectural rework, it just can't work 100% reliably, so we ended up
disabling that code by default.

An approach using a separate thread for input was also tried, but the
overhead and complexity of proper locking between display drivers and
input drivers killed that idea too.

>
> Index: xenocara/driver/xf86-input-ws/src/ws.c
> ===================================================================
> RCS file: /cvs/xenocara/driver/xf86-input-ws/src/ws.c,v
> retrieving revision 1.57
> diff -u -p -r1.57 ws.c
> --- xenocara/driver/xf86-input-ws/src/ws.c 8 Jul 2012 14:22:03 -0000 1.57
> +++ xenocara/driver/xf86-input-ws/src/ws.c 7 Jul 2013 18:33:57 -0000
> @@ -474,7 +474,7 @@ wsReadInput(InputInfoPtr pInfo)
>  {
>   WSDevicePtr priv = (WSDevicePtr)pInfo->private;
>   static struct wscons_event eventList[NUMEVENTS];
> - int n, c;
> + int n, c, dx, dy;
>   struct wscons_event *event = eventList;
>   unsigned char *pBuf;
>  
> @@ -488,10 +488,11 @@ wsReadInput(InputInfoPtr pInfo)
>   if (n == 0)
>   return;
>  
> + dx = dy = 0;
>   n /= sizeof(struct wscons_event);
>   while (n--) {
>   int buttons = priv->lastButtons;
> - int dx = 0, dy = 0, dz = 0, dw = 0, ax = 0, ay = 0;
> + int dz = 0, dw = 0, ax = 0, ay = 0;
>   int zbutton = 0, wbutton = 0;
>  
>   switch (event->type) {
> @@ -506,11 +507,11 @@ wsReadInput(InputInfoPtr pInfo)
>      buttons));
>   break;
>   case WSCONS_EVENT_MOUSE_DELTA_X:
> - dx = event->value;
> + dx += event->value;
>   DBG(4, ErrorF("Relative X %d\n", event->value));
>   break;
>   case WSCONS_EVENT_MOUSE_DELTA_Y:
> - dy = -event->value;
> + dy -= event->value;
>   DBG(4, ErrorF("Relative Y %d\n", event->value));
>   break;
>   case WSCONS_EVENT_MOUSE_ABSOLUTE_X:
> @@ -548,14 +549,18 @@ wsReadInput(InputInfoPtr pInfo)
>   }
>   ++event;
>  
> - if (dx || dy) {
> - if (wsWheelEmuFilterMotion(pInfo, dx, dy))
> + if ((dx || dy) && event->type != WSCONS_EVENT_MOUSE_DELTA_X &&
> +    event->type != WSCONS_EVENT_MOUSE_DELTA_Y) {
> + int tmpx = dx, tmpy = dy;
> + dx = dy = 0;
> +
> + if (wsWheelEmuFilterMotion(pInfo, tmpx, tmpy))
>   continue;
>  
>   /* relative motion event */
>   DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
> -    dx, dy));
> - xf86PostMotionEvent(pInfo->dev, 0, 0, 2, dx, dy);
> +    tmpx, tmpy));
> + xf86PostMotionEvent(pInfo->dev, 0, 0, 2, tmpx, tmpy);
>   }
>   if (dz && priv->Z.negative != WS_NOMAP
>      && priv->Z.positive != WS_NOMAP) {
> @@ -583,9 +588,9 @@ wsReadInput(InputInfoPtr pInfo)
>   ay = tmp;
>   }
>   if (ax) {
> - dx = ax - priv->old_ax;
> + int xdelta = ax - priv->old_ax;
>   priv->old_ax = ax;
> - if (wsWheelEmuFilterMotion(pInfo, dx, 0))
> + if (wsWheelEmuFilterMotion(pInfo, xdelta, 0))
>   continue;
>  
>   /* absolute position event */
> @@ -593,15 +598,24 @@ wsReadInput(InputInfoPtr pInfo)
>   xf86PostMotionEvent(pInfo->dev, 1, 0, 1, ax);
>   }
>   if (ay) {
> - dy = ay - priv->old_ay;
> + int ydelta = ay - priv->old_ay;
>   priv->old_ay = ay;
> - if (wsWheelEmuFilterMotion(pInfo, 0, dy))
> + if (wsWheelEmuFilterMotion(pInfo, 0, ydelta))
>   continue;
>  
>   /* absolute position event */
>   DBG(3, ErrorF("postMotionEvent y %d\n", ay));
>   xf86PostMotionEvent(pInfo->dev, 1, 1, 1, ay);
>   }
> + }
> + if (dx || dy) {
> + if (wsWheelEmuFilterMotion(pInfo, dx, dy))
> + return;
> +
> + /* relative motion event */
> + DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
> +    dx, dy));
> + xf86PostMotionEvent(pInfo->dev, 0, 0, 2, dx, dy);
>   }
>   return;
>  }
>
>
>

--
Matthieu Herrb

Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

Henri Kemppainen-2
> The issue that input drivers devices need high refresh frequency to be
> able to achieve high-precision freehand drawing is quite well known¹.

Yes.  But the bug here isn't about that.  I think I'll have to elaborate
a little.

When you move the mouse, it will report its motion in an event with a
two-component vector.  Naturally both components must always be present,
otherwise information is lost.  When wsmouse(4) gets such an event,
it will break that event into two separate events, one for the X delta
and another for Y.  This is because the current event structure cannot
contain the deltas for both axes.

Now ws(4) takes these two events and naturally considers them independent
(as there is no metadata to couple them together), and fills in the blanks
by setting one or the other delta to zero.  The result is two orthogonal
vectors which represent a motion your mouse never reported.  Their sum is
the original vector, but the motion they describe is still wrong.

So the problem here is not refresh frequency, precision or anything like
that.  It's just that our data gets effectively mangled and corrupted
between the two layers.  My diff attempts to reconstruct the original
vector by summing consecutive delta events, but as you noted below, this
can also result in data loss.  The lossless method would involve extending
the ws event structure or adding some metadata (in the form of new events),
or perhaps stuffing both deltas into the value field we have now (this is
lossless only if they fit in 16 bits)..  Interpolation is something I
definitely wouldn't apply here :-)

> I re-did a few experiments with and without your patch on my laptop
> (with the Lenovo track point wihch is a relative device). I could not
> reproduce the waves you're getting in Gimp, but without your patch
> diagonals show indeed larger staircases effects. Under xfig(1) or
> bitmap(1) the difference is much less noticeable. Clearly the
> strategy used by applications to trac mouse motion for freehand
> drawing are not the same.
>
> I do fear that with some devices your patch will collapse too many
> events and make it harder to follow small radius curves.

Right, I did not consider this case.  If this is a problem, perhaps
the code could be changed to only collapse a pair of DELTA_X and
DELTA_Y events, but never more than that.  This way it might still
incorrectly merge two independent motions along the axes into one
diagonal motion, but I would expect that to be rare, and the deltas
to be so small that it has very little impact on anything.

Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

Henri Kemppainen-2
> > I do fear that with some devices your patch will collapse too many
> > events and make it harder to follow small radius curves.
>
> Right, I did not consider this case.  If this is a problem, perhaps
> the code could be changed to only collapse a pair of DELTA_X and
> DELTA_Y events, but never more than that.  This way it might still
> incorrectly merge two independent motions along the axes into one
> diagonal motion, but I would expect that to be rare, and the deltas
> to be so small that it has very little impact on anything.

Here's a diff that does just that.  If ws receives more than one
delta along an axis, it will not sum these; each will go in a separate
event.  But if it gets an X delta followed by an Y delta (or vice versa),
these will be combined into a single event.

Index: xenocara/driver/xf86-input-ws/src/ws.c
===================================================================
RCS file: /cvs/xenocara/driver/xf86-input-ws/src/ws.c,v
retrieving revision 1.57
diff -u -p -r1.57 ws.c
--- xenocara/driver/xf86-input-ws/src/ws.c 8 Jul 2012 14:22:03 -0000 1.57
+++ xenocara/driver/xf86-input-ws/src/ws.c 8 Jul 2013 17:12:27 -0000
@@ -474,7 +474,7 @@ wsReadInput(InputInfoPtr pInfo)
 {
  WSDevicePtr priv = (WSDevicePtr)pInfo->private;
  static struct wscons_event eventList[NUMEVENTS];
- int n, c;
+ int n, c, dx, dy;
  struct wscons_event *event = eventList;
  unsigned char *pBuf;
 
@@ -488,10 +488,11 @@ wsReadInput(InputInfoPtr pInfo)
  if (n == 0)
  return;
 
+ dx = dy = 0;
  n /= sizeof(struct wscons_event);
  while (n--) {
  int buttons = priv->lastButtons;
- int dx = 0, dy = 0, dz = 0, dw = 0, ax = 0, ay = 0;
+ int newdx = 0, newdy = 0, dz = 0, dw = 0, ax = 0, ay = 0;
  int zbutton = 0, wbutton = 0;
 
  switch (event->type) {
@@ -506,11 +507,17 @@ wsReadInput(InputInfoPtr pInfo)
     buttons));
  break;
  case WSCONS_EVENT_MOUSE_DELTA_X:
- dx = event->value;
+ if (!dx)
+ dx = event->value;
+ else
+ newdx = event->value;
  DBG(4, ErrorF("Relative X %d\n", event->value));
  break;
  case WSCONS_EVENT_MOUSE_DELTA_Y:
- dy = -event->value;
+ if (!dy)
+ dy = -event->value;
+ else
+ newdy = -event->value;
  DBG(4, ErrorF("Relative Y %d\n", event->value));
  break;
  case WSCONS_EVENT_MOUSE_ABSOLUTE_X:
@@ -548,14 +555,20 @@ wsReadInput(InputInfoPtr pInfo)
  }
  ++event;
 
- if (dx || dy) {
- if (wsWheelEmuFilterMotion(pInfo, dx, dy))
+ if ((newdx || newdy) || ((dx || dy) &&
+    event->type != WSCONS_EVENT_MOUSE_DELTA_X &&
+    event->type != WSCONS_EVENT_MOUSE_DELTA_Y)) {
+ int tmpx = dx, tmpy = dy;
+ dx = newdx;
+ dy = newdy;
+
+ if (wsWheelEmuFilterMotion(pInfo, tmpx, tmpy))
  continue;
 
  /* relative motion event */
  DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
-    dx, dy));
- xf86PostMotionEvent(pInfo->dev, 0, 0, 2, dx, dy);
+    tmpx, tmpy));
+ xf86PostMotionEvent(pInfo->dev, 0, 0, 2, tmpx, tmpy);
  }
  if (dz && priv->Z.negative != WS_NOMAP
     && priv->Z.positive != WS_NOMAP) {
@@ -583,9 +596,9 @@ wsReadInput(InputInfoPtr pInfo)
  ay = tmp;
  }
  if (ax) {
- dx = ax - priv->old_ax;
+ int xdelta = ax - priv->old_ax;
  priv->old_ax = ax;
- if (wsWheelEmuFilterMotion(pInfo, dx, 0))
+ if (wsWheelEmuFilterMotion(pInfo, xdelta, 0))
  continue;
 
  /* absolute position event */
@@ -593,15 +606,24 @@ wsReadInput(InputInfoPtr pInfo)
  xf86PostMotionEvent(pInfo->dev, 1, 0, 1, ax);
  }
  if (ay) {
- dy = ay - priv->old_ay;
+ int ydelta = ay - priv->old_ay;
  priv->old_ay = ay;
- if (wsWheelEmuFilterMotion(pInfo, 0, dy))
+ if (wsWheelEmuFilterMotion(pInfo, 0, ydelta))
  continue;
 
  /* absolute position event */
  DBG(3, ErrorF("postMotionEvent y %d\n", ay));
  xf86PostMotionEvent(pInfo->dev, 1, 1, 1, ay);
  }
+ }
+ if (dx || dy) {
+ if (wsWheelEmuFilterMotion(pInfo, dx, dy))
+ return;
+
+ /* relative motion event */
+ DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
+    dx, dy));
+ xf86PostMotionEvent(pInfo->dev, 0, 0, 2, dx, dy);
  }
  return;
 }

Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

Edd Barrett
Hi,

On Mon, Jul 08, 2013 at 08:26:56PM +0300, Henri Kemppainen wrote:
> Here's a diff that does just that.  If ws receives more than one
> delta along an axis, it will not sum these; each will go in a separate
> event.  But if it gets an X delta followed by an Y delta (or vice versa),
> these will be combined into a single event.

I have also been experiencing these jagged staircase lines on my x230t.
FWIW here are the outcomes of my experiments:

Prior to applying your diff (the most recent one):
Touchpad: smooth lines.
Nipple:   jagged lines.
Pen:      jagged lines.

After applying your diff:
Touchpad: smooth lines.
Nipple:   smooth lines.
Pen:      jagged lines.

I wonder if it is because the pen is an absolute pointing device.  You
probably need extra magic in the WSCONS_EVENT_MOUSE_ABSOLUTE_* cases.

--
Best Regards
Edd Barrett

http://www.theunixzoo.co.uk

Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

Matthieu Herrb
In reply to this post by Henri Kemppainen-2
On Mon, Jul 08, 2013 at 08:26:56PM +0300, Henri Kemppainen wrote:

> > > I do fear that with some devices your patch will collapse too many
> > > events and make it harder to follow small radius curves.
> >
> > Right, I did not consider this case.  If this is a problem, perhaps
> > the code could be changed to only collapse a pair of DELTA_X and
> > DELTA_Y events, but never more than that.  This way it might still
> > incorrectly merge two independent motions along the axes into one
> > diagonal motion, but I would expect that to be rare, and the deltas
> > to be so small that it has very little impact on anything.
>
> Here's a diff that does just that.  If ws receives more than one
> delta along an axis, it will not sum these; each will go in a separate
> event.  But if it gets an X delta followed by an Y delta (or vice versa),
> these will be combined into a single event.

I've committed this patch. Thanks again for the contribution.

>
> Index: xenocara/driver/xf86-input-ws/src/ws.c
> ===================================================================
> RCS file: /cvs/xenocara/driver/xf86-input-ws/src/ws.c,v
> retrieving revision 1.57
> diff -u -p -r1.57 ws.c
> --- xenocara/driver/xf86-input-ws/src/ws.c 8 Jul 2012 14:22:03 -0000 1.57
> +++ xenocara/driver/xf86-input-ws/src/ws.c 8 Jul 2013 17:12:27 -0000
> @@ -474,7 +474,7 @@ wsReadInput(InputInfoPtr pInfo)
>  {
>   WSDevicePtr priv = (WSDevicePtr)pInfo->private;
>   static struct wscons_event eventList[NUMEVENTS];
> - int n, c;
> + int n, c, dx, dy;
>   struct wscons_event *event = eventList;
>   unsigned char *pBuf;
>  
> @@ -488,10 +488,11 @@ wsReadInput(InputInfoPtr pInfo)
>   if (n == 0)
>   return;
>  
> + dx = dy = 0;
>   n /= sizeof(struct wscons_event);
>   while (n--) {
>   int buttons = priv->lastButtons;
> - int dx = 0, dy = 0, dz = 0, dw = 0, ax = 0, ay = 0;
> + int newdx = 0, newdy = 0, dz = 0, dw = 0, ax = 0, ay = 0;
>   int zbutton = 0, wbutton = 0;
>  
>   switch (event->type) {
> @@ -506,11 +507,17 @@ wsReadInput(InputInfoPtr pInfo)
>      buttons));
>   break;
>   case WSCONS_EVENT_MOUSE_DELTA_X:
> - dx = event->value;
> + if (!dx)
> + dx = event->value;
> + else
> + newdx = event->value;
>   DBG(4, ErrorF("Relative X %d\n", event->value));
>   break;
>   case WSCONS_EVENT_MOUSE_DELTA_Y:
> - dy = -event->value;
> + if (!dy)
> + dy = -event->value;
> + else
> + newdy = -event->value;
>   DBG(4, ErrorF("Relative Y %d\n", event->value));
>   break;
>   case WSCONS_EVENT_MOUSE_ABSOLUTE_X:
> @@ -548,14 +555,20 @@ wsReadInput(InputInfoPtr pInfo)
>   }
>   ++event;
>  
> - if (dx || dy) {
> - if (wsWheelEmuFilterMotion(pInfo, dx, dy))
> + if ((newdx || newdy) || ((dx || dy) &&
> +    event->type != WSCONS_EVENT_MOUSE_DELTA_X &&
> +    event->type != WSCONS_EVENT_MOUSE_DELTA_Y)) {
> + int tmpx = dx, tmpy = dy;
> + dx = newdx;
> + dy = newdy;
> +
> + if (wsWheelEmuFilterMotion(pInfo, tmpx, tmpy))
>   continue;
>  
>   /* relative motion event */
>   DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
> -    dx, dy));
> - xf86PostMotionEvent(pInfo->dev, 0, 0, 2, dx, dy);
> +    tmpx, tmpy));
> + xf86PostMotionEvent(pInfo->dev, 0, 0, 2, tmpx, tmpy);
>   }
>   if (dz && priv->Z.negative != WS_NOMAP
>      && priv->Z.positive != WS_NOMAP) {
> @@ -583,9 +596,9 @@ wsReadInput(InputInfoPtr pInfo)
>   ay = tmp;
>   }
>   if (ax) {
> - dx = ax - priv->old_ax;
> + int xdelta = ax - priv->old_ax;
>   priv->old_ax = ax;
> - if (wsWheelEmuFilterMotion(pInfo, dx, 0))
> + if (wsWheelEmuFilterMotion(pInfo, xdelta, 0))
>   continue;
>  
>   /* absolute position event */
> @@ -593,15 +606,24 @@ wsReadInput(InputInfoPtr pInfo)
>   xf86PostMotionEvent(pInfo->dev, 1, 0, 1, ax);
>   }
>   if (ay) {
> - dy = ay - priv->old_ay;
> + int ydelta = ay - priv->old_ay;
>   priv->old_ay = ay;
> - if (wsWheelEmuFilterMotion(pInfo, 0, dy))
> + if (wsWheelEmuFilterMotion(pInfo, 0, ydelta))
>   continue;
>  
>   /* absolute position event */
>   DBG(3, ErrorF("postMotionEvent y %d\n", ay));
>   xf86PostMotionEvent(pInfo->dev, 1, 1, 1, ay);
>   }
> + }
> + if (dx || dy) {
> + if (wsWheelEmuFilterMotion(pInfo, dx, dy))
> + return;
> +
> + /* relative motion event */
> + DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
> +    dx, dy));
> + xf86PostMotionEvent(pInfo->dev, 0, 0, 2, dx, dy);
>   }
>   return;
>  }
>

--
Matthieu Herrb

Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

Edd Barrett
In reply to this post by Edd Barrett
On Thu, Jul 18, 2013 at 09:23:00PM +0100, Edd Barrett wrote:
> After applying your diff:
> Touchpad: smooth lines.
> Nipple:   smooth lines.
> Pen:      jagged lines.
>
> I wonder if it is because the pen is an absolute pointing device.  You
> probably need extra magic in the WSCONS_EVENT_MOUSE_ABSOLUTE_* cases.

The following diff fixes the jagged diagonal lines for absolute pointing
devices. I think I might be able to simplify some of the relative pointer
code here too, but that should be a separate diff.

Tested on my x230t and will continue to test. No regrssions noticed on
relative pointing devices.

OK?

Index: src/ws.c
===================================================================
RCS file: /home/edd/cvsync/cvs/xenocara/driver/xf86-input-ws/src/ws.c,v
retrieving revision 1.58
diff -u -p -u -r1.58 ws.c
--- src/ws.c 20 Jul 2013 13:24:50 -0000 1.58
+++ src/ws.c 16 Oct 2013 22:36:21 -0000
@@ -474,7 +474,7 @@ wsReadInput(InputInfoPtr pInfo)
 {
  WSDevicePtr priv = (WSDevicePtr)pInfo->private;
  static struct wscons_event eventList[NUMEVENTS];
- int n, c, dx, dy;
+ int n, c, dx, dy, ax, ay;
  struct wscons_event *event = eventList;
  unsigned char *pBuf;
 
@@ -488,11 +488,11 @@ wsReadInput(InputInfoPtr pInfo)
  if (n == 0)
  return;
 
- dx = dy = 0;
+ dx = dy = ax = ay = 0;
  n /= sizeof(struct wscons_event);
  while (n--) {
  int buttons = priv->lastButtons;
- int newdx = 0, newdy = 0, dz = 0, dw = 0, ax = 0, ay = 0;
+ int newdx = 0, newdy = 0, dz = 0, dw = 0;
  int zbutton = 0, wbutton = 0;
 
  switch (event->type) {
@@ -595,25 +595,17 @@ wsReadInput(InputInfoPtr pInfo)
  ax = ay;
  ay = tmp;
  }
- if (ax) {
+ if (ax && ay) {
  int xdelta = ax - priv->old_ax;
- priv->old_ax = ax;
- if (wsWheelEmuFilterMotion(pInfo, xdelta, 0))
- continue;
-
- /* absolute position event */
- DBG(3, ErrorF("postMotionEvent X %d\n", ax));
- xf86PostMotionEvent(pInfo->dev, 1, 0, 1, ax);
- }
- if (ay) {
  int ydelta = ay - priv->old_ay;
+ priv->old_ax = ax;
  priv->old_ay = ay;
- if (wsWheelEmuFilterMotion(pInfo, 0, ydelta))
+ if (wsWheelEmuFilterMotion(pInfo, xdelta, ydelta))
  continue;
-
  /* absolute position event */
- DBG(3, ErrorF("postMotionEvent y %d\n", ay));
- xf86PostMotionEvent(pInfo->dev, 1, 1, 1, ay);
+ DBG(3, ErrorF("postMotionEvent X %d Y %d\n", ax, ay));
+ xf86PostMotionEvent(pInfo->dev, 1, 0, 2, ax, ay);
+ ax = ay = 0; /* prevent second post below */
  }
  }
  if (dx || dy) {
@@ -624,6 +616,24 @@ wsReadInput(InputInfoPtr pInfo)
  DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
     dx, dy));
  xf86PostMotionEvent(pInfo->dev, 0, 0, 2, dx, dy);
+ }
+ if (ax) {
+ int xdelta = ax - priv->old_ax;
+ priv->old_ax = ax;
+ if (wsWheelEmuFilterMotion(pInfo, xdelta, 0))
+ return;
+ /* absolute position event */
+ DBG(3, ErrorF("postMotionEvent X %d\n", ax));
+ xf86PostMotionEvent(pInfo->dev, 1, 0, 1, ax);
+ }
+ if (ay) {
+ int ydelta = ay - priv->old_ay;
+ priv->old_ay = ay;
+ if (wsWheelEmuFilterMotion(pInfo, 0, ydelta))
+ return;
+ /* absolute position event */
+ DBG(3, ErrorF("postMotionEvent Y %d\n", ay));
+ xf86PostMotionEvent(pInfo->dev, 1, 1, 1, ay);
  }
  return;
 }

--
Best Regards
Edd Barrett

http://www.theunixzoo.co.uk

Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

Edd Barrett
On Wed, Oct 16, 2013 at 11:45:34PM +0100, Edd Barrett wrote:
> Tested on my x230t and will continue to test. No regrssions noticed on
> relative pointing devices.
>
> OK?

Anyone?

I appreciate that I am probably the only one using OpenBSD on a tablet,
but a "looks OK" and "no regressions for relative pointing devices"
would be great.

Keen to get this in so that I don't have to rebuild the ws driver every
time I update my snapshot.

--
Best Regards
Edd Barrett

http://www.theunixzoo.co.uk

Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

Henri Kemppainen-2
> > Tested on my x230t and will continue to test. No regrssions noticed on
> > relative pointing devices.
> >
> > OK?
>
> Anyone?
>
> I appreciate that I am probably the only one using OpenBSD on a tablet,
> but a "looks OK" and "no regressions for relative pointing devices"
> would be great.

What happens when priv->swap_axes is set, and the ax && ay branch is
taken along with the wsWheelEmuFilterMotion() branch.  Following
continue another event is processed and now the axes are swapped again
(ax and ay were not reset after use) and then what?  Not very likely
I know.

In hindsight it's possible that wheel emulation has been broken in
the previous revision; a glance at wsWheelEmuFilterMotion suggests
it doesn't like to handle both x and y axes at once, and will only
do the former (resetting the latter) if both are set.  I've never
used this thing though, and I didn't dive deeper.

Other than that I guess your diff is about as reasonable as one
can be with this hairy code.  I was waiting and hoping someone
else with an absolute pointing device could've checked and tested
it because I really didn't enjoy working on this file when I did
my patch :-)

Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

Edd Barrett
On Thu, Oct 24, 2013 at 10:33:22PM +0300, Henri Kemppainen wrote:
> What happens when priv->swap_axes is set, and the ax && ay branch is
> taken along with the wsWheelEmuFilterMotion() branch.  Following
> continue another event is processed and now the axes are swapped again
> (ax and ay were not reset after use) and then what?  Not very likely
> I know.

Ah, yes, there is the possibility of posting an inconsistent pointer sample
in this case. Perhaps we should only update the old_ax/old_ay if the
wsWheelEmuFilterMotion branch is not taken?

What do you think? And yes, this is a very very unlikely case. You could
argue it wouldn't matter even if it did happen.

--
Best Regards
Edd Barrett

http://www.theunixzoo.co.uk

Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

Alexander Shadchin
On Fri, Oct 25, 2013 at 11:41:25AM +0100, Edd Barrett wrote:

> On Thu, Oct 24, 2013 at 10:33:22PM +0300, Henri Kemppainen wrote:
> > What happens when priv->swap_axes is set, and the ax && ay branch is
> > taken along with the wsWheelEmuFilterMotion() branch.  Following
> > continue another event is processed and now the axes are swapped again
> > (ax and ay were not reset after use) and then what?  Not very likely
> > I know.
>
> Ah, yes, there is the possibility of posting an inconsistent pointer sample
> in this case. Perhaps we should only update the old_ax/old_ay if the
> wsWheelEmuFilterMotion branch is not taken?
>
> What do you think? And yes, this is a very very unlikely case. You could
> argue it wouldn't matter even if it did happen.
>
> --
> Best Regards
> Edd Barrett
>
> http://www.theunixzoo.co.uk
>
Alternative solution without extra magic (need rebuild kernel).

Before (on example pms(4)):
* user move mouse
* pms(4) read state mouse and process it
* pms(4) send dx, dy and buttons in wscons
* wscons generate simple events
* ws(4) reads one event and process it immediately

After applying diff:
* user move mouse
* pms(4) read state mouse and process it
* pms(4) send dx, dy and buttons in wscons
* wscons generate simple events and adds SYNC event
* ws(4) reads events until it receives SYNC, and only then begins processing

Tested on mouse.

Comments ?

PS:
synaptics(4) is working on a similar basis

--
Alexandr Shadchin


kernel.diff (2K) Download Attachment
ws.diff (9K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

Henri Kemppainen-2
> From: Alexandr Shadchin
>
> Before (on example pms(4)):
> * user move mouse
> * pms(4) read state mouse and process it
> * pms(4) send dx, dy and buttons in wscons
> * wscons generate simple events
> * ws(4) reads one event and process it immediately
>
> After applying diff:
> * user move mouse
> * pms(4) read state mouse and process it
> * pms(4) send dx, dy and buttons in wscons
> * wscons generate simple events and adds SYNC event
> * ws(4) reads events until it receives SYNC, and only then begins processing
>
> Tested on mouse.
>
> Comments ?
>
> PS:
> synaptics(4) is working on a similar basis

Absolutely yes for this.  This is one of the approaches I originally
considered, but then feared it'd be too intrusive.  I didn't realize
WS_INPUT_SYNC was already a thing and that we're doing this with
synaptics.  This'll also fix another downside which I mentioned with the
previous approach (that it could join two unrelated x & y events if they
follow each other).

I'm busy crossing the time_t chasm and compiling ports because the
packages on mirrors haven't gotten across the libstdc++ bump (damnit).
I'll try take a better look at your diff and test it tomorrow.

Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

Henri Kemppainen-2
In reply to this post by Alexander Shadchin
> Alternative solution without extra magic (need rebuild kernel).
>
> Before (on example pms(4)):
> * user move mouse
> * pms(4) read state mouse and process it
> * pms(4) send dx, dy and buttons in wscons
> * wscons generate simple events
> * ws(4) reads one event and process it immediately
>
> After applying diff:
> * user move mouse
> * pms(4) read state mouse and process it
> * pms(4) send dx, dy and buttons in wscons
> * wscons generate simple events and adds SYNC event
> * ws(4) reads events until it receives SYNC, and only then begins processing
>
> Tested on mouse.
>
> Comments ?

Testing with usb mouse, so far so good.  Mouse wheel emulation seems to be
working fine as well.  The code reads ok to me, and it's definitely a lot
cleaner than the old magic.

Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

Matthieu Herrb
In reply to this post by Alexander Shadchin
On Sat, Oct 26, 2013 at 10:13:11PM +0600, Alexandr Shadchin wrote:

> On Fri, Oct 25, 2013 at 11:41:25AM +0100, Edd Barrett wrote:
> > On Thu, Oct 24, 2013 at 10:33:22PM +0300, Henri Kemppainen wrote:
> > > What happens when priv->swap_axes is set, and the ax && ay branch is
> > > taken along with the wsWheelEmuFilterMotion() branch.  Following
> > > continue another event is processed and now the axes are swapped again
> > > (ax and ay were not reset after use) and then what?  Not very likely
> > > I know.
> >
> > Ah, yes, there is the possibility of posting an inconsistent pointer sample
> > in this case. Perhaps we should only update the old_ax/old_ay if the
> > wsWheelEmuFilterMotion branch is not taken?
> >
> > What do you think? And yes, this is a very very unlikely case. You could
> > argue it wouldn't matter even if it did happen.
> >
> > --
> > Best Regards
> > Edd Barrett
> >
> > http://www.theunixzoo.co.uk
> >
>
> Alternative solution without extra magic (need rebuild kernel).
>
> Before (on example pms(4)):
> * user move mouse
> * pms(4) read state mouse and process it
> * pms(4) send dx, dy and buttons in wscons
> * wscons generate simple events
> * ws(4) reads one event and process it immediately
>
> After applying diff:
> * user move mouse
> * pms(4) read state mouse and process it
> * pms(4) send dx, dy and buttons in wscons
> * wscons generate simple events and adds SYNC event
> * ws(4) reads events until it receives SYNC, and only then begins processing
>
> Tested on mouse.
>
> Comments ?

Look good to me. However I've a concern about compatibility with
NetBSD. The kernel change should be documented in the commit message
for xf86-input-ws so that they can catch up with the kernel change
before they update xf86-input-ws...

>
> PS:
> synaptics(4) is working on a similar basis
>
> --
> Alexandr Shadchin
>

> Index: dev/pckbc/pms.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pckbc/pms.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 pms.c
> --- dev/pckbc/pms.c 20 Sep 2013 14:07:30 -0000 1.48
> +++ dev/pckbc/pms.c 26 Oct 2013 15:09:53 -0000
> @@ -1155,8 +1155,7 @@ pms_proc_synaptics(struct pms_softc *sc)
>   if (syn->wsmode == WSMOUSE_NATIVE) {
>   wsmouse_input(sc->sc_wsmousedev, buttons, x, y, z, w,
>      WSMOUSE_INPUT_ABSOLUTE_X | WSMOUSE_INPUT_ABSOLUTE_Y |
> -    WSMOUSE_INPUT_ABSOLUTE_Z | WSMOUSE_INPUT_ABSOLUTE_W |
> -    WSMOUSE_INPUT_SYNC);
> +    WSMOUSE_INPUT_ABSOLUTE_Z | WSMOUSE_INPUT_ABSOLUTE_W);
>   } else {
>   dx = dy = 0;
>   if (z > SYNAPTICS_PRESSURE) {
> @@ -1470,8 +1469,7 @@ pms_proc_alps(struct pms_softc *sc)
>  
>   wsmouse_input(sc->sc_wsmousedev, buttons, x, y, z, w,
>      WSMOUSE_INPUT_ABSOLUTE_X | WSMOUSE_INPUT_ABSOLUTE_Y |
> -    WSMOUSE_INPUT_ABSOLUTE_Z | WSMOUSE_INPUT_ABSOLUTE_W |
> -    WSMOUSE_INPUT_SYNC);
> +    WSMOUSE_INPUT_ABSOLUTE_Z | WSMOUSE_INPUT_ABSOLUTE_W);
>  
>   alps->old_fin = fin;
>   } else {
> @@ -2321,8 +2319,7 @@ elantech_send_input(struct pms_softc *sc
>      WSMOUSE_INPUT_ABSOLUTE_X |
>      WSMOUSE_INPUT_ABSOLUTE_Y |
>      WSMOUSE_INPUT_ABSOLUTE_Z |
> -    WSMOUSE_INPUT_ABSOLUTE_W |
> -    WSMOUSE_INPUT_SYNC);
> +    WSMOUSE_INPUT_ABSOLUTE_W);
>   } else {
>   dx = dy = 0;
>  
> Index: dev/wscons/wsmouse.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/wscons/wsmouse.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 wsmouse.c
> --- dev/wscons/wsmouse.c 18 Oct 2013 13:54:09 -0000 1.24
> +++ dev/wscons/wsmouse.c 26 Oct 2013 15:09:55 -0000
> @@ -452,13 +452,11 @@ wsmouse_input(struct device *wsmousedev,
>   ub ^= d;
>   }
>  
> - if (flags & WSMOUSE_INPUT_SYNC) {
> - NEXT;
> - ev->type = WSCONS_EVENT_SYNC;
> - ev->value = 0;
> - TIMESTAMP;
> - ADVANCE;
> - }
> + NEXT;
> + ev->type = WSCONS_EVENT_SYNC;
> + ev->value = 0;
> + TIMESTAMP;
> + ADVANCE;
>  
>   /* XXX fake wscons_event notifying wsmoused(8) to close mouse device */
>   if (flags & WSMOUSE_INPUT_WSMOUSED_CLOSE) {
> Index: dev/wscons/wsmousevar.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/wscons/wsmousevar.h,v
> retrieving revision 1.6
> diff -u -p -r1.6 wsmousevar.h
> --- dev/wscons/wsmousevar.h 22 Jul 2012 18:28:36 -0000 1.6
> +++ dev/wscons/wsmousevar.h 26 Oct 2013 15:09:55 -0000
> @@ -74,7 +74,6 @@ int wsmousedevprint(void *, const char *
>  #define WSMOUSE_INPUT_WSMOUSED_CLOSE (1<<3) /* notify wsmoused(8) to close
>    mouse device */
>  #define WSMOUSE_INPUT_ABSOLUTE_W (1<<4)
> -#define WSMOUSE_INPUT_SYNC (1<<5)
>  
>  void wsmouse_input(struct device *kbddev, u_int btns,
>     int x, int y, int z, int w, u_int flags);

> Index: ws.c
> ===================================================================
> RCS file: /cvs/xenocara/driver/xf86-input-ws/src/ws.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 ws.c
> --- ws.c 20 Jul 2013 13:24:50 -0000 1.58
> +++ ws.c 26 Oct 2013 16:03:01 -0000
> @@ -428,13 +428,6 @@ wsDeviceOn(DeviceIntPtr pWS)
>   }
>   }
>   }
> - priv->buffer = XisbNew(pInfo->fd,
> -    sizeof(struct wscons_event) * NUMEVENTS);
> - if (priv->buffer == NULL) {
> - xf86IDrvMsg(pInfo, X_ERROR, "cannot alloc xisb buffer\n");
> - wsClose(pInfo);
> - return !Success;
> - }
>   xf86AddEnabledDevice(pInfo);
>   wsmbEmuOn(pInfo);
>   pWS->public.on = TRUE;
> @@ -462,170 +455,150 @@ wsDeviceOff(DeviceIntPtr pWS)
>   xf86RemoveEnabledDevice(pInfo);
>   wsClose(pInfo);
>   }
> - if (priv->buffer) {
> - XisbFree(priv->buffer);
> - priv->buffer = NULL;
> - }
>   pWS->public.on = FALSE;
>  }
>  
> -static void
> -wsReadInput(InputInfoPtr pInfo)
> +static Bool
> +wsReadEvent(InputInfoPtr pInfo, struct wscons_event *event)
>  {
> - WSDevicePtr priv = (WSDevicePtr)pInfo->private;
> - static struct wscons_event eventList[NUMEVENTS];
> - int n, c, dx, dy;
> - struct wscons_event *event = eventList;
> - unsigned char *pBuf;
> -
> - XisbBlockDuration(priv->buffer, -1);
> - pBuf = (unsigned char *)eventList;
> - n = 0;
> - while (n < sizeof(eventList) && (c = XisbRead(priv->buffer)) >= 0) {
> - pBuf[n++] = (unsigned char)c;
> + Bool rc = TRUE;
> + ssize_t len;
> +
> + len = read(pInfo->fd, event, sizeof(struct wscons_event));
> + if (len <= 0) {
> + if (errno != EAGAIN)
> + xf86IDrvMsg(pInfo, X_ERROR, "read error %s\n",
> +    strerror(errno));
> + rc = FALSE;
> + } else if (len != sizeof(struct wscons_event)) {
> + xf86IDrvMsg(pInfo, X_ERROR,
> +    "read error, invalid number of bytes\n");
> + rc = FALSE;
>   }
>  
> - if (n == 0)
> - return;
> + return rc;
> +}
>  
> - dx = dy = 0;
> - n /= sizeof(struct wscons_event);
> - while (n--) {
> - int buttons = priv->lastButtons;
> - int newdx = 0, newdy = 0, dz = 0, dw = 0, ax = 0, ay = 0;
> - int zbutton = 0, wbutton = 0;
> +static Bool
> +wsReadHwState(InputInfoPtr pInfo, wsHwState *hw)
> +{
> + WSDevicePtr priv = (WSDevicePtr)pInfo->private;
> + struct wscons_event event;
> +
> + bzero(hw, sizeof(wsHwState));
> +
> + hw->buttons = priv->lastButtons;
> + hw->ax = priv->old_ax;
> + hw->ay = priv->old_ay;
>  
> - switch (event->type) {
> + while (wsReadEvent(pInfo, &event)) {
> + switch (event.type) {
>   case WSCONS_EVENT_MOUSE_UP:
> - buttons &= ~(1 << event->value);
> - DBG(4, ErrorF("Button %d up %x\n", event->value,
> -    buttons));
> + hw->buttons &= ~(1 << event.value);
> + DBG(4, ErrorF("Button %d up %x\n", event.value,
> +    hw->buttons));
>   break;
>   case WSCONS_EVENT_MOUSE_DOWN:
> - buttons |= (1 << event->value);
> - DBG(4, ErrorF("Button %d down %x\n", event->value,
> -    buttons));
> + hw->buttons |= (1 << event.value);
> + DBG(4, ErrorF("Button %d down %x\n", event.value,
> +    hw->buttons));
>   break;
>   case WSCONS_EVENT_MOUSE_DELTA_X:
> - if (!dx)
> - dx = event->value;
> - else
> - newdx = event->value;
> - DBG(4, ErrorF("Relative X %d\n", event->value));
> + hw->dx = event.value;
> + DBG(4, ErrorF("Relative X %d\n", event.value));
>   break;
>   case WSCONS_EVENT_MOUSE_DELTA_Y:
> - if (!dy)
> - dy = -event->value;
> - else
> - newdy = -event->value;
> - DBG(4, ErrorF("Relative Y %d\n", event->value));
> + hw->dy = -event.value;
> + DBG(4, ErrorF("Relative Y %d\n", event.value));
> + break;
> + case WSCONS_EVENT_MOUSE_DELTA_Z:
> + hw->dz = event.value;
> + DBG(4, ErrorF("Relative Z %d\n", event.value));
> + break;
> + case WSCONS_EVENT_MOUSE_DELTA_W:
> + hw->dw = event.value;
> + DBG(4, ErrorF("Relative W %d\n", event.value));
>   break;
>   case WSCONS_EVENT_MOUSE_ABSOLUTE_X:
> - DBG(4, ErrorF("Absolute X %d\n", event->value));
> - if (event->value == 4095)
> - break;
> - ax = event->value;
> + hw->ax = event.value;
>   if (priv->inv_x)
> - ax = priv->max_x - ax + priv->min_x;
> + hw->ax = priv->max_x - hw->ax + priv->min_x;
> + DBG(4, ErrorF("Absolute X %d\n", event.value));
>   break;
>   case WSCONS_EVENT_MOUSE_ABSOLUTE_Y:
> - DBG(4, ErrorF("Absolute Y %d\n", event->value));
> - ay = event->value;
> + hw->ay = event.value;
>   if (priv->inv_y)
> - ay = priv->max_y - ay + priv->min_y;
> - break;
> - case WSCONS_EVENT_MOUSE_DELTA_Z:
> - DBG(4, ErrorF("Relative Z %d\n", event->value));
> - dz = event->value;
> + hw->ay = priv->max_y - hw->ay + priv->min_y;
> + DBG(4, ErrorF("Absolute Y %d\n", event.value));
>   break;
>   case WSCONS_EVENT_MOUSE_ABSOLUTE_Z:
> + case WSCONS_EVENT_MOUSE_ABSOLUTE_W:
>   /* ignore those */
> - ++event;
>   continue;
> - break;
> - case WSCONS_EVENT_MOUSE_DELTA_W:
> - DBG(4, ErrorF("Relative W %d\n", event->value));
> - dw = event->value;
> - break;
> + case WSCONS_EVENT_SYNC:
> + DBG(4, ErrorF("Sync\n"));
> + return TRUE;
>   default:
>   xf86IDrvMsg(pInfo, X_WARNING,
> -    "bad wsmouse event type=%d\n", event->type);
> - ++event;
> +    "bad wsmouse event type=%d\n", event.type);
>   continue;
>   }
> - ++event;
> + }
>  
> - if ((newdx || newdy) || ((dx || dy) &&
> -    event->type != WSCONS_EVENT_MOUSE_DELTA_X &&
> -    event->type != WSCONS_EVENT_MOUSE_DELTA_Y)) {
> - int tmpx = dx, tmpy = dy;
> - dx = newdx;
> - dy = newdy;
> -
> - if (wsWheelEmuFilterMotion(pInfo, tmpx, tmpy))
> - continue;
> -
> - /* relative motion event */
> - DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
> -    tmpx, tmpy));
> - xf86PostMotionEvent(pInfo->dev, 0, 0, 2, tmpx, tmpy);
> - }
> - if (dz && priv->Z.negative != WS_NOMAP
> -    && priv->Z.positive != WS_NOMAP) {
> - zbutton = (dz < 0) ? priv->Z.negative :
> -    priv->Z.positive;
> - DBG(4, ErrorF("Z -> button %d\n", zbutton));
> - wsButtonClicks(pInfo, zbutton, abs(dz));
> - }
> - if (dw && priv->W.negative != WS_NOMAP
> -    && priv->W.positive != WS_NOMAP) {
> - wbutton = (dw < 0) ? priv->W.negative :
> -    priv->W.positive;
> - DBG(4, ErrorF("W -> button %d\n", wbutton));
> - wsButtonClicks(pInfo, wbutton, abs(dw));
> - }
> - if (priv->lastButtons != buttons) {
> - /* button event */
> - wsSendButtons(pInfo, buttons);
> - }
> - if (priv->swap_axes) {
> - int tmp;
> + return FALSE;
> +}
>  
> - tmp = ax;
> - ax = ay;
> - ay = tmp;
> - }
> - if (ax) {
> - int xdelta = ax - priv->old_ax;
> - priv->old_ax = ax;
> - if (wsWheelEmuFilterMotion(pInfo, xdelta, 0))
> - continue;
> -
> - /* absolute position event */
> - DBG(3, ErrorF("postMotionEvent X %d\n", ax));
> - xf86PostMotionEvent(pInfo->dev, 1, 0, 1, ax);
> - }
> - if (ay) {
> - int ydelta = ay - priv->old_ay;
> - priv->old_ay = ay;
> - if (wsWheelEmuFilterMotion(pInfo, 0, ydelta))
> - continue;
> -
> - /* absolute position event */
> - DBG(3, ErrorF("postMotionEvent y %d\n", ay));
> - xf86PostMotionEvent(pInfo->dev, 1, 1, 1, ay);
> - }
> +static void
> +wsReadInput(InputInfoPtr pInfo)
> +{
> + WSDevicePtr priv = (WSDevicePtr)pInfo->private;
> + wsHwState hw;
> +
> + if (!wsReadHwState(pInfo, &hw))
> + return;
> +
> + if ((hw.dx || hw.dy) && !wsWheelEmuFilterMotion(pInfo, hw.dx, hw.dy)) {
> + /* relative motion event */
> + DBG(3, ErrorF("postMotionEvent dX %d dY %d\n", hw.dx, hw.dy));
> + xf86PostMotionEvent(pInfo->dev, 0, 0, 2, hw.dx, hw.dy);
>   }
> - if (dx || dy) {
> - if (wsWheelEmuFilterMotion(pInfo, dx, dy))
> + if (hw.dz && priv->Z.negative != WS_NOMAP
> +    && priv->Z.positive != WS_NOMAP) {
> + int zbutton;
> + zbutton = (hw.dz < 0) ? priv->Z.negative : priv->Z.positive;
> + DBG(4, ErrorF("Z -> button %d (%d)\n", zbutton, abs(hw.dz)));
> + wsButtonClicks(pInfo, zbutton, abs(hw.dz));
> + }
> + if (hw.dw && priv->W.negative != WS_NOMAP
> +    && priv->W.positive != WS_NOMAP) {
> + int wbutton;
> + wbutton = (hw.dw < 0) ? priv->W.negative : priv->W.positive;
> + DBG(4, ErrorF("W -> button %d (%d)\n", wbutton, abs(hw.dw)));
> + wsButtonClicks(pInfo, wbutton, abs(hw.dw));
> + }
> + if (priv->lastButtons != hw.buttons) {
> + /* button event */
> + wsSendButtons(pInfo, hw.buttons);
> + }
> + if (priv->swap_axes) {
> + int tmp;
> +
> + tmp = hw.ax;
> + hw.ax = hw.ay;
> + hw.ay = tmp;
> + }
> + if ((hw.ax != priv->old_ax) || (hw.ay != priv->old_ay)) {
> + int xdelta = hw.ax - priv->old_ax;
> + int ydelta = hw.ay - priv->old_ay;
> + priv->old_ax = hw.ax;
> + priv->old_ay = hw.ay;
> + if (wsWheelEmuFilterMotion(pInfo, xdelta, ydelta))
>   return;
>  
> - /* relative motion event */
> - DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
> -    dx, dy));
> - xf86PostMotionEvent(pInfo->dev, 0, 0, 2, dx, dy);
> + /* absolute position event */
> + DBG(3, ErrorF("postMotionEvent X %d Y %d\n", hw.ax, hw.ay));
> + xf86PostMotionEvent(pInfo->dev, 1, 0, 2, hw.ax, hw.ay);
>   }
> - return;
>  }
>  
>  static void
> Index: ws.h
> ===================================================================
> RCS file: /cvs/xenocara/driver/xf86-input-ws/src/ws.h,v
> retrieving revision 1.12
> diff -u -p -r1.12 ws.h
> --- ws.h 12 Jun 2012 17:44:56 -0000 1.12
> +++ ws.h 26 Oct 2013 16:03:01 -0000
> @@ -29,7 +29,6 @@ extern int ws_debug_level;
>  #define NAXES 2 /* X and Y axes only */
>  #define NBUTTONS 32 /* max theoretical buttons */
>  #define DFLTBUTTONS 3 /* default number of buttons */
> -#define NUMEVENTS 16 /* max # of ws events to read at once */
>  
>  #define WS_NOMAP 0
>  
> @@ -40,6 +39,12 @@ typedef struct {
>   int traveled_distance;
>  } WheelAxis, *WheelAxisPtr;
>  
> +typedef struct {
> + unsigned int buttons;
> + int dx, dy, dz, dw;
> + int ax, ay;
> +} wsHwState;
> +
>  typedef struct WSDevice {
>   char *devName; /* device name */
>   int type; /* ws device type */
> @@ -50,7 +55,6 @@ typedef struct WSDevice {
>   int raw;
>   int inv_x, inv_y;
>   int screen_no;
> - pointer buffer;
>   WheelAxis Z;
>   WheelAxis W;
>   struct wsmouse_calibcoords coords; /* mirror of the kernel values */


--
Matthieu Herrb

Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

Alexander Shadchin
On Sun, Oct 27, 2013 at 04:31:37PM +0100, Matthieu Herrb wrote:

> On Sat, Oct 26, 2013 at 10:13:11PM +0600, Alexandr Shadchin wrote:
> > On Fri, Oct 25, 2013 at 11:41:25AM +0100, Edd Barrett wrote:
> > > On Thu, Oct 24, 2013 at 10:33:22PM +0300, Henri Kemppainen wrote:
> > > > What happens when priv->swap_axes is set, and the ax && ay branch is
> > > > taken along with the wsWheelEmuFilterMotion() branch.  Following
> > > > continue another event is processed and now the axes are swapped again
> > > > (ax and ay were not reset after use) and then what?  Not very likely
> > > > I know.
> > >
> > > Ah, yes, there is the possibility of posting an inconsistent pointer sample
> > > in this case. Perhaps we should only update the old_ax/old_ay if the
> > > wsWheelEmuFilterMotion branch is not taken?
> > >
> > > What do you think? And yes, this is a very very unlikely case. You could
> > > argue it wouldn't matter even if it did happen.
> > >
> > > --
> > > Best Regards
> > > Edd Barrett
> > >
> > > http://www.theunixzoo.co.uk
> > >
> >
> > Alternative solution without extra magic (need rebuild kernel).
> >
> > Before (on example pms(4)):
> > * user move mouse
> > * pms(4) read state mouse and process it
> > * pms(4) send dx, dy and buttons in wscons
> > * wscons generate simple events
> > * ws(4) reads one event and process it immediately
> >
> > After applying diff:
> > * user move mouse
> > * pms(4) read state mouse and process it
> > * pms(4) send dx, dy and buttons in wscons
> > * wscons generate simple events and adds SYNC event
> > * ws(4) reads events until it receives SYNC, and only then begins processing
> >
> > Tested on mouse.
> >
> > Comments ?
>
> Look good to me. However I've a concern about compatibility with
> NetBSD. The kernel change should be documented in the commit message
> for xf86-input-ws so that they can catch up with the kernel change
> before they update xf86-input-ws...
>

Update diff (add small hack for NetBSD).

--
Alexandr Shadchin

Index: ws.c
===================================================================
RCS file: /cvs/xenocara/driver/xf86-input-ws/src/ws.c,v
retrieving revision 1.58
diff -u -p -r1.58 ws.c
--- ws.c 20 Jul 2013 13:24:50 -0000 1.58
+++ ws.c 29 Oct 2013 16:57:05 -0000
@@ -428,13 +428,6 @@ wsDeviceOn(DeviceIntPtr pWS)
  }
  }
  }
- priv->buffer = XisbNew(pInfo->fd,
-    sizeof(struct wscons_event) * NUMEVENTS);
- if (priv->buffer == NULL) {
- xf86IDrvMsg(pInfo, X_ERROR, "cannot alloc xisb buffer\n");
- wsClose(pInfo);
- return !Success;
- }
  xf86AddEnabledDevice(pInfo);
  wsmbEmuOn(pInfo);
  pWS->public.on = TRUE;
@@ -462,170 +455,157 @@ wsDeviceOff(DeviceIntPtr pWS)
  xf86RemoveEnabledDevice(pInfo);
  wsClose(pInfo);
  }
- if (priv->buffer) {
- XisbFree(priv->buffer);
- priv->buffer = NULL;
- }
  pWS->public.on = FALSE;
 }
 
-static void
-wsReadInput(InputInfoPtr pInfo)
+static Bool
+wsReadEvent(InputInfoPtr pInfo, struct wscons_event *event)
 {
- WSDevicePtr priv = (WSDevicePtr)pInfo->private;
- static struct wscons_event eventList[NUMEVENTS];
- int n, c, dx, dy;
- struct wscons_event *event = eventList;
- unsigned char *pBuf;
-
- XisbBlockDuration(priv->buffer, -1);
- pBuf = (unsigned char *)eventList;
- n = 0;
- while (n < sizeof(eventList) && (c = XisbRead(priv->buffer)) >= 0) {
- pBuf[n++] = (unsigned char)c;
+ Bool rc = TRUE;
+ ssize_t len;
+
+ len = read(pInfo->fd, event, sizeof(struct wscons_event));
+ if (len <= 0) {
+ if (errno != EAGAIN)
+ xf86IDrvMsg(pInfo, X_ERROR, "read error %s\n",
+    strerror(errno));
+ rc = FALSE;
+ } else if (len != sizeof(struct wscons_event)) {
+ xf86IDrvMsg(pInfo, X_ERROR,
+    "read error, invalid number of bytes\n");
+ rc = FALSE;
  }
 
- if (n == 0)
- return;
+ return rc;
+}
 
- dx = dy = 0;
- n /= sizeof(struct wscons_event);
- while (n--) {
- int buttons = priv->lastButtons;
- int newdx = 0, newdy = 0, dz = 0, dw = 0, ax = 0, ay = 0;
- int zbutton = 0, wbutton = 0;
+static Bool
+wsReadHwState(InputInfoPtr pInfo, wsHwState *hw)
+{
+ WSDevicePtr priv = (WSDevicePtr)pInfo->private;
+ struct wscons_event event;
+
+ bzero(hw, sizeof(wsHwState));
+
+ hw->buttons = priv->lastButtons;
+ hw->ax = priv->old_ax;
+ hw->ay = priv->old_ay;
 
- switch (event->type) {
+ while (wsReadEvent(pInfo, &event)) {
+ switch (event.type) {
  case WSCONS_EVENT_MOUSE_UP:
- buttons &= ~(1 << event->value);
- DBG(4, ErrorF("Button %d up %x\n", event->value,
-    buttons));
+ hw->buttons &= ~(1 << event.value);
+ DBG(4, ErrorF("Button %d up %x\n", event.value,
+    hw->buttons));
  break;
  case WSCONS_EVENT_MOUSE_DOWN:
- buttons |= (1 << event->value);
- DBG(4, ErrorF("Button %d down %x\n", event->value,
-    buttons));
+ hw->buttons |= (1 << event.value);
+ DBG(4, ErrorF("Button %d down %x\n", event.value,
+    hw->buttons));
  break;
  case WSCONS_EVENT_MOUSE_DELTA_X:
- if (!dx)
- dx = event->value;
- else
- newdx = event->value;
- DBG(4, ErrorF("Relative X %d\n", event->value));
+ hw->dx = event.value;
+ DBG(4, ErrorF("Relative X %d\n", event.value));
  break;
  case WSCONS_EVENT_MOUSE_DELTA_Y:
- if (!dy)
- dy = -event->value;
- else
- newdy = -event->value;
- DBG(4, ErrorF("Relative Y %d\n", event->value));
+ hw->dy = -event.value;
+ DBG(4, ErrorF("Relative Y %d\n", event.value));
+ break;
+ case WSCONS_EVENT_MOUSE_DELTA_Z:
+ hw->dz = event.value;
+ DBG(4, ErrorF("Relative Z %d\n", event.value));
+ break;
+ case WSCONS_EVENT_MOUSE_DELTA_W:
+ hw->dw = event.value;
+ DBG(4, ErrorF("Relative W %d\n", event.value));
  break;
  case WSCONS_EVENT_MOUSE_ABSOLUTE_X:
- DBG(4, ErrorF("Absolute X %d\n", event->value));
- if (event->value == 4095)
- break;
- ax = event->value;
+ hw->ax = event.value;
  if (priv->inv_x)
- ax = priv->max_x - ax + priv->min_x;
+ hw->ax = priv->max_x - hw->ax + priv->min_x;
+ DBG(4, ErrorF("Absolute X %d\n", event.value));
  break;
  case WSCONS_EVENT_MOUSE_ABSOLUTE_Y:
- DBG(4, ErrorF("Absolute Y %d\n", event->value));
- ay = event->value;
+ hw->ay = event.value;
  if (priv->inv_y)
- ay = priv->max_y - ay + priv->min_y;
- break;
- case WSCONS_EVENT_MOUSE_DELTA_Z:
- DBG(4, ErrorF("Relative Z %d\n", event->value));
- dz = event->value;
+ hw->ay = priv->max_y - hw->ay + priv->min_y;
+ DBG(4, ErrorF("Absolute Y %d\n", event.value));
  break;
  case WSCONS_EVENT_MOUSE_ABSOLUTE_Z:
+ case WSCONS_EVENT_MOUSE_ABSOLUTE_W:
  /* ignore those */
- ++event;
  continue;
- break;
- case WSCONS_EVENT_MOUSE_DELTA_W:
- DBG(4, ErrorF("Relative W %d\n", event->value));
- dw = event->value;
- break;
+ case WSCONS_EVENT_SYNC:
+ DBG(4, ErrorF("Sync\n"));
+ return TRUE;
  default:
  xf86IDrvMsg(pInfo, X_WARNING,
-    "bad wsmouse event type=%d\n", event->type);
- ++event;
+    "bad wsmouse event type=%d\n", event.type);
  continue;
  }
- ++event;
+#ifdef __NetBSD__
+ /*
+ * XXX NetBSD reads only one event.
+ * WSCONS_EVENT_SYNC is not supported yet.
+ */
+ return TRUE;
+#endif
+ }
 
- if ((newdx || newdy) || ((dx || dy) &&
-    event->type != WSCONS_EVENT_MOUSE_DELTA_X &&
-    event->type != WSCONS_EVENT_MOUSE_DELTA_Y)) {
- int tmpx = dx, tmpy = dy;
- dx = newdx;
- dy = newdy;
-
- if (wsWheelEmuFilterMotion(pInfo, tmpx, tmpy))
- continue;
-
- /* relative motion event */
- DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
-    tmpx, tmpy));
- xf86PostMotionEvent(pInfo->dev, 0, 0, 2, tmpx, tmpy);
- }
- if (dz && priv->Z.negative != WS_NOMAP
-    && priv->Z.positive != WS_NOMAP) {
- zbutton = (dz < 0) ? priv->Z.negative :
-    priv->Z.positive;
- DBG(4, ErrorF("Z -> button %d\n", zbutton));
- wsButtonClicks(pInfo, zbutton, abs(dz));
- }
- if (dw && priv->W.negative != WS_NOMAP
-    && priv->W.positive != WS_NOMAP) {
- wbutton = (dw < 0) ? priv->W.negative :
-    priv->W.positive;
- DBG(4, ErrorF("W -> button %d\n", wbutton));
- wsButtonClicks(pInfo, wbutton, abs(dw));
- }
- if (priv->lastButtons != buttons) {
- /* button event */
- wsSendButtons(pInfo, buttons);
- }
- if (priv->swap_axes) {
- int tmp;
+ return FALSE;
+}
 
- tmp = ax;
- ax = ay;
- ay = tmp;
- }
- if (ax) {
- int xdelta = ax - priv->old_ax;
- priv->old_ax = ax;
- if (wsWheelEmuFilterMotion(pInfo, xdelta, 0))
- continue;
-
- /* absolute position event */
- DBG(3, ErrorF("postMotionEvent X %d\n", ax));
- xf86PostMotionEvent(pInfo->dev, 1, 0, 1, ax);
- }
- if (ay) {
- int ydelta = ay - priv->old_ay;
- priv->old_ay = ay;
- if (wsWheelEmuFilterMotion(pInfo, 0, ydelta))
- continue;
-
- /* absolute position event */
- DBG(3, ErrorF("postMotionEvent y %d\n", ay));
- xf86PostMotionEvent(pInfo->dev, 1, 1, 1, ay);
- }
+static void
+wsReadInput(InputInfoPtr pInfo)
+{
+ WSDevicePtr priv = (WSDevicePtr)pInfo->private;
+ wsHwState hw;
+
+ if (!wsReadHwState(pInfo, &hw))
+ return;
+
+ if ((hw.dx || hw.dy) && !wsWheelEmuFilterMotion(pInfo, hw.dx, hw.dy)) {
+ /* relative motion event */
+ DBG(3, ErrorF("postMotionEvent dX %d dY %d\n", hw.dx, hw.dy));
+ xf86PostMotionEvent(pInfo->dev, 0, 0, 2, hw.dx, hw.dy);
  }
- if (dx || dy) {
- if (wsWheelEmuFilterMotion(pInfo, dx, dy))
+ if (hw.dz && priv->Z.negative != WS_NOMAP
+    && priv->Z.positive != WS_NOMAP) {
+ int zbutton;
+ zbutton = (hw.dz < 0) ? priv->Z.negative : priv->Z.positive;
+ DBG(4, ErrorF("Z -> button %d (%d)\n", zbutton, abs(hw.dz)));
+ wsButtonClicks(pInfo, zbutton, abs(hw.dz));
+ }
+ if (hw.dw && priv->W.negative != WS_NOMAP
+    && priv->W.positive != WS_NOMAP) {
+ int wbutton;
+ wbutton = (hw.dw < 0) ? priv->W.negative : priv->W.positive;
+ DBG(4, ErrorF("W -> button %d (%d)\n", wbutton, abs(hw.dw)));
+ wsButtonClicks(pInfo, wbutton, abs(hw.dw));
+ }
+ if (priv->lastButtons != hw.buttons) {
+ /* button event */
+ wsSendButtons(pInfo, hw.buttons);
+ }
+ if (priv->swap_axes) {
+ int tmp;
+
+ tmp = hw.ax;
+ hw.ax = hw.ay;
+ hw.ay = tmp;
+ }
+ if ((hw.ax != priv->old_ax) || (hw.ay != priv->old_ay)) {
+ int xdelta = hw.ax - priv->old_ax;
+ int ydelta = hw.ay - priv->old_ay;
+ priv->old_ax = hw.ax;
+ priv->old_ay = hw.ay;
+ if (wsWheelEmuFilterMotion(pInfo, xdelta, ydelta))
  return;
 
- /* relative motion event */
- DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
-    dx, dy));
- xf86PostMotionEvent(pInfo->dev, 0, 0, 2, dx, dy);
+ /* absolute position event */
+ DBG(3, ErrorF("postMotionEvent X %d Y %d\n", hw.ax, hw.ay));
+ xf86PostMotionEvent(pInfo->dev, 1, 0, 2, hw.ax, hw.ay);
  }
- return;
 }
 
 static void
Index: ws.h
===================================================================
RCS file: /cvs/xenocara/driver/xf86-input-ws/src/ws.h,v
retrieving revision 1.12
diff -u -p -r1.12 ws.h
--- ws.h 12 Jun 2012 17:44:56 -0000 1.12
+++ ws.h 29 Oct 2013 16:57:05 -0000
@@ -29,7 +29,6 @@ extern int ws_debug_level;
 #define NAXES 2 /* X and Y axes only */
 #define NBUTTONS 32 /* max theoretical buttons */
 #define DFLTBUTTONS 3 /* default number of buttons */
-#define NUMEVENTS 16 /* max # of ws events to read at once */
 
 #define WS_NOMAP 0
 
@@ -40,6 +39,12 @@ typedef struct {
  int traveled_distance;
 } WheelAxis, *WheelAxisPtr;
 
+typedef struct {
+ unsigned int buttons;
+ int dx, dy, dz, dw;
+ int ax, ay;
+} wsHwState;
+
 typedef struct WSDevice {
  char *devName; /* device name */
  int type; /* ws device type */
@@ -50,7 +55,6 @@ typedef struct WSDevice {
  int raw;
  int inv_x, inv_y;
  int screen_no;
- pointer buffer;
  WheelAxis Z;
  WheelAxis W;
  struct wsmouse_calibcoords coords; /* mirror of the kernel values */

Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

Alf Schlichting
In reply to this post by Alexander Shadchin
On Sat, Oct 26, 2013 at 10:13:11PM +0600, Alexandr Shadchin wrote:

> On Fri, Oct 25, 2013 at 11:41:25AM +0100, Edd Barrett wrote:
> > On Thu, Oct 24, 2013 at 10:33:22PM +0300, Henri Kemppainen wrote:
> > > What happens when priv->swap_axes is set, and the ax && ay branch is
> > > taken along with the wsWheelEmuFilterMotion() branch.  Following
> > > continue another event is processed and now the axes are swapped again
> > > (ax and ay were not reset after use) and then what?  Not very likely
> > > I know.
> >
> > Ah, yes, there is the possibility of posting an inconsistent pointer sample
> > in this case. Perhaps we should only update the old_ax/old_ay if the
> > wsWheelEmuFilterMotion branch is not taken?
> >
> > What do you think? And yes, this is a very very unlikely case. You could
> > argue it wouldn't matter even if it did happen.
> >
> > --
> > Best Regards
> > Edd Barrett
> >
> > http://www.theunixzoo.co.uk
> >
>
> Alternative solution without extra magic (need rebuild kernel).
>
> Before (on example pms(4)):
> * user move mouse
> * pms(4) read state mouse and process it
> * pms(4) send dx, dy and buttons in wscons
> * wscons generate simple events
> * ws(4) reads one event and process it immediately
>
> After applying diff:
> * user move mouse
> * pms(4) read state mouse and process it
> * pms(4) send dx, dy and buttons in wscons
> * wscons generate simple events and adds SYNC event
> * ws(4) reads events until it receives SYNC, and only then begins processing
>
> Tested on mouse.
>
> Comments ?
>
> PS:
> synaptics(4) is working on a similar basis
>
> --
> Alexandr Shadchin
>

I've tested this diff with some hours of QuakeWorld, an old
and very fast/demanding FPS game. Works well.
This staircase fix + KMS have improved these types of games on
OpenBSD a lot.

However, mouse input still isn't there yet, it still feels
slightly delayed and not precise enough compared to running
the same game and game client on Linux. This is only subtle
and will most likely not be noticed without playing quite a lot
though.
Modern Quake engines rely on high sleep granuality so I always
have to rip out any usleep from the clients since in OpenBSD
atm you can't sleep below tic rate (roughly 1/100 sec on my machine)
AFAIK.
This may add to the aforementioned felt latency on input.

Alf

OpenBSD 5.4-current (GENERIC.MP) #0: Tue Oct 29 14:32:26 CET 2013
    [hidden email]:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 8555986944 (8159MB)
avail mem = 8320094208 (7934MB)
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.4 @ 0xf0100 (37 entries)
bios0: vendor Award Software International, Inc. version "F9" date 03/21/2012
bios0: Gigabyte Technology Co., Ltd. PH67A-UD3-B3
acpi0 at bios0: rev 0
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP MSDM HPET MCFG ASPT SSPT EUDS MATS TAMG APIC SSDT MATS
acpi0: wakeup devices PEX0(S5) PEX1(S5) PEX2(S5) PEX3(S5) PEX4(S5) PEX5(S5) PEX6(S5) PEX7(S5) HUB0(S5) UAR1(S3) USBE(S3) USE2(S3) AZAL(S5) PCI0(S5)
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpihpet0 at acpi0: 14318179 Hz
acpimcfg0 at acpi0 addr 0xf4000000, bus 0-63
acpimadt0 at acpi0 addr 0xfee00000: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM) i5-2500 CPU @ 3.30GHz, 3396.09 MHz
cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC
cpu0: 256KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.1.0, IBE
cpu1 at mainbus0: apid 2 (application processor)
cpu1: Intel(R) Core(TM) i5-2500 CPU @ 3.30GHz, 3395.56 MHz
cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 4 (application processor)
cpu2: Intel(R) Core(TM) i5-2500 CPU @ 3.30GHz, 3395.56 MHz
cpu2: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC
cpu2: 256KB 64b/line 8-way L2 cache
cpu2: smt 0, core 2, package 0
cpu3 at mainbus0: apid 6 (application processor)
cpu3: Intel(R) Core(TM) i5-2500 CPU @ 3.30GHz, 3395.56 MHz
cpu3: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC
cpu3: 256KB 64b/line 8-way L2 cache
cpu3: smt 0, core 3, package 0
ioapic0 at mainbus0: apid 2 pa 0xfec00000, version 20, 24 pins
ioapic0: misconfigured as apic 0, remapped to apid 2
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus 1 (PEG0)
acpiprt2 at acpi0: bus -1 (PEG1)
acpiprt3 at acpi0: bus 2 (PEX0)
acpiprt4 at acpi0: bus 3 (PEX1)
acpiprt5 at acpi0: bus 4 (PEX2)
acpiprt6 at acpi0: bus 5 (PEX3)
acpiprt7 at acpi0: bus -1 (PEX4)
acpiprt8 at acpi0: bus -1 (PEX5)
acpiprt9 at acpi0: bus -1 (PEX6)
acpiprt10 at acpi0: bus -1 (PEX7)
acpiprt11 at acpi0: bus -1 (HUB0)
acpicpu0 at acpi0: C3, C2, C1, PSS
acpicpu1 at acpi0: C3, C2, C1, PSS
acpicpu2 at acpi0: C3, C2, C1, PSS
acpicpu3 at acpi0: C3, C2, C1, PSS
acpibtn0 at acpi0: PWRB
cpu0: Enhanced SpeedStep 3396 MHz: speeds: 3601, 3600, 3500, 3400, 3300, 3200, 3100, 3000, 2900, 2800, 2700, 2600, 2500, 2400, 2300, 2200, 2100, 2000, 1600 MHz
pci0 at mainbus0 bus 0
pchb0 at pci0 dev 0 function 0 "Intel Core 2G Host" rev 0x09
ppb0 at pci0 dev 1 function 0 "Intel Core 2G PCIE" rev 0x09: msi
pci1 at ppb0 bus 1
radeondrm0 at pci1 dev 0 function 0 "ATI Radeon HD 6770" rev 0x00: apic 2 int 16
drm0 at radeondrm0
azalia0 at pci1 dev 0 function 1 "ATI Radeon HD 5700 Audio" rev 0x00: msi
azalia0: no supported codecs
"Intel 6 Series MEI" rev 0x04 at pci0 dev 22 function 0 not configured
ehci0 at pci0 dev 26 function 0 "Intel 6 Series USB" rev 0x05: apic 2 int 18
usb0 at ehci0: USB revision 2.0
uhub0 at usb0 "Intel EHCI root hub" rev 2.00/1.00 addr 1
azalia1 at pci0 dev 27 function 0 "Intel 6 Series HD Audio" rev 0x05: msi
azalia1: codecs: Realtek/0x0889
audio0 at azalia1
ppb1 at pci0 dev 28 function 0 "Intel 6 Series PCIE" rev 0xb5: msi
pci2 at ppb1 bus 2
ppb2 at pci0 dev 28 function 1 "Intel 6 Series PCIE" rev 0xb5: msi
pci3 at ppb2 bus 3
"NEC xHCI" rev 0x04 at pci3 dev 0 function 0 not configured
ppb3 at pci0 dev 28 function 2 "Intel 6 Series PCIE" rev 0xb5: msi
pci4 at ppb3 bus 4
re0 at pci4 dev 0 function 0 "Realtek 8168" rev 0x06: RTL8168E/8111E (0x2c00), apic 2 int 18, address 1c:6f:65:c9:e1:d6
rgephy0 at re0 phy 7: RTL8169S/8110S PHY, rev. 4
ppb4 at pci0 dev 28 function 3 "Intel 82801BA Hub-to-PCI" rev 0xb5: msi
pci5 at ppb4 bus 5
ppb5 at pci5 dev 0 function 0 "ITExpress IT8892E PCIE-PCI" rev 0x10
pci6 at ppb5 bus 6
"Creative Labs SoundBlaster Audigy LS" rev 0x00 at pci6 dev 1 function 0 not configured
ehci1 at pci0 dev 29 function 0 "Intel 6 Series USB" rev 0x05: apic 2 int 23
usb1 at ehci1: USB revision 2.0
uhub1 at usb1 "Intel EHCI root hub" rev 2.00/1.00 addr 1
pcib0 at pci0 dev 31 function 0 "Intel H67 LPC" rev 0x05
pciide0 at pci0 dev 31 function 2 "Intel 6 Series SATA" rev 0x05: DMA, channel 0 configured to native-PCI, channel 1 configured to native-PCI
pciide0: using apic 2 int 19 for native-PCI interrupt
wd0 at pciide0 channel 0 drive 0: <SAMSUNG SSD 830 Series>
wd0: 16-sector PIO, LBA48, 122104MB, 250069680 sectors
wd1 at pciide0 channel 0 drive 1: <Samsung SSD 840 Series>
wd1: 16-sector PIO, LBA48, 114473MB, 234441648 sectors
wd0(pciide0:0:0): using PIO mode 4, Ultra-DMA mode 6
wd1(pciide0:0:1): using PIO mode 4, Ultra-DMA mode 6
wd2 at pciide0 channel 1 drive 0: <Samsung SSD 840 Series>
wd2: 16-sector PIO, LBA48, 114473MB, 234441648 sectors
wd2(pciide0:1:0): using PIO mode 4, Ultra-DMA mode 6
ichiic0 at pci0 dev 31 function 3 "Intel 6 Series SMBus" rev 0x05: apic 2 int 18
iic0 at ichiic0
iic0: addr 0x4e 03=cc 04=cc 06=0d 07=01 08=0f 0a=40 0c=ae 0d=fa 0e=c6 0f=88 12=ff 13=ff 20=14 26=03 28=83 29=12 2a=23 2c=10 2f=e4 3c=28 3e=01 47=99 49=ff 4a=ff 4b=ff 4c=ff 4d=ff 4e=ff 4f=ff 71=ff 80=ff 82=ff 84=ff 86=ff 90=ff 91=ff 92=ff 93=ff 94=ff 95=ff 96=ff 98=ff 99=ff 9a=ff 9b=ff 9c=ff 9d=ff 9e=ff a0=ff a1=ff a2=ff a3=ff a4=ff a5=ff a6=ff a8=ff a9=ff aa=ff ab=ff ac=ff ad=ff ae=ff f0=ff f1=30 f2=2c f3=07 f4=e5 f5=3f f6=0c f7=fe f8=3c words 00=0000 01=0000 02=00cc 03=cccc 04=cc00 05=000d 06=0d01 07=010f
spdmem0 at iic0 addr 0x50: 4GB DDR3 SDRAM PC3-10600
spdmem1 at iic0 addr 0x52: 4GB DDR3 SDRAM PC3-10600
pciide1 at pci0 dev 31 function 5 "Intel 6 Series SATA" rev 0x05: DMA, channel 0 wired to native-PCI, channel 1 wired to native-PCI
pciide1: using apic 2 int 19 for native-PCI interrupt
wd3 at pciide1 channel 0 drive 0: <Samsung SSD 840 Series>
wd3: 16-sector PIO, LBA48, 238475MB, 488397168 sectors
wd3(pciide1:0:0): using PIO mode 4, Ultra-DMA mode 6
isa0 at pcib0
isadma0 at isa0
com0 at isa0 port 0x3f8/8 irq 4: ns16550a, 16 byte fifo
pckbc0 at isa0 port 0x60/5
pcppi0 at isa0 port 0x61
spkr0 at pcppi0
it0 at isa0 port 0x2e/2: IT8728F rev 0, EC port 0x290
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
uhub2 at uhub0 port 1 "Intel Rate Matching Hub" rev 2.00/0.00 addr 2
uhidev0 at uhub2 port 1 configuration 1 interface 0 "Logitech USB-PS/2 Optical Mouse" rev 2.00/22.00 addr 3
uhidev0: iclass 3/1
ums0 at uhidev0: 8 buttons, Z dir
wsmouse0 at ums0 mux 0
uhidev1 at uhub2 port 3 configuration 1 interface 0 "Metadot - Das Keyboard Das Keyboard Model S" rev 1.10/1.00 addr 4
uhidev1: iclass 3/1
ukbd0 at uhidev1: 8 variable keys, 6 key codes
wskbd0 at ukbd0: console keyboard
uhidev2 at uhub2 port 3 configuration 1 interface 1 "Metadot - Das Keyboard Das Keyboard Model S" rev 1.10/1.00 addr 4
uhidev2: iclass 3/0, 3 report ids
uhid0 at uhidev2 reportid 2: input=1, output=0, feature=0
uhid1 at uhidev2 reportid 3: input=3, output=0, feature=0
uhub3 at uhub2 port 4 "NEC hub" rev 2.00/1.00 addr 5
uhub4 at uhub1 port 1 "Intel Rate Matching Hub" rev 2.00/0.00 addr 2
vscsi0 at root
scsibus0 at vscsi0: 256 targets
softraid0 at root
scsibus1 at softraid0: 256 targets
root on wd1a (d336e3eb55d9041d.a) swap on wd1b dump on wd1b
drm: initializing kernel modesetting (JUNIPER 0x1002:0x68BA 0x1787:0x200A).
radeondrm0: VRAM: 1024M 0x0000000000000000 - 0x000000003FFFFFFF (1024M used)
radeondrm0: GTT: 512M 0x0000000040000000 - 0x000000005FFFFFFF
ttm_pool_mm_shrink_init stub
drm: probing gen 2 caps for device 0x8086:0x0101 = 2/0
drm: enabling PCIE gen 2 link speeds, disable with radeon.pcie_gen2=0
drm: PCIE GART of 512M enabled (table at 0x0000000000040000).
drm: Internal thermal controller with fan control
error: [drm:pid0:drm_edid_block_valid] *ERROR* EDID checksum is invalid, remainder is 254
Raw EDID:

00 01 04 00 fe 5b 80 a0  70 38 35 40 30 20 35 00
13 2a 21 00 00 1a 86 6f  80 a0 70 38 40 40 30 20
35 00 13 2a 21 00 00 1a  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 2c
error: [drm:pid0:drm_edid_block_valid] *ERROR* EDID checksum is invalid, remainder is 254
Raw EDID:

00 01 04 00 fe 5b 80 a0  70 38 35 40 30 20 35 00
13 2a 21 00 00 1a 86 6f  80 a0 70 38 40 40 30 20
35 00 13 2a 21 00 00 1a  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 2c
error: [drm:pid0:drm_edid_block_valid] *ERROR* EDID checksum is invalid, remainder is 254
Raw EDID:

00 01 04 00 fe 5b 80 a0  70 38 35 40 30 20 35 00
13 2a 21 00 00 1a 86 6f  80 a0 70 38 40 40 30 20
35 00 13 2a 21 00 00 1a  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 2c
error: [drm:pid0:drm_edid_block_valid] *ERROR* EDID checksum is invalid, remainder is 254
Raw EDID:

00 01 04 00 fe 5b 80 a0  70 38 35 40 30 20 35 00
13 2a 21 00 00 1a 86 6f  80 a0 70 38 40 40 30 20
35 00 13 2a 21 00 00 1a  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 2c
DVI-I-1: Ignoring invalid EDID block 1.
radeondrm0: 1280x1024
wsdisplay0 at radeondrm0 mux 1: console (std, vt100 emulation), using wskbd0
wsdisplay0: screen 1-5 added (std, vt100 emulation)

Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

Edd Barrett
In reply to this post by Alexander Shadchin
On Tue, Oct 29, 2013 at 11:07:29PM +0600, Alexandr Shadchin wrote:
>
> > Look good to me. However I've a concern about compatibility with
> > NetBSD. The kernel change should be documented in the commit message
> > for xf86-input-ws so that they can catch up with the kernel change
> > before they update xf86-input-ws...
> >
>
> Update diff (add small hack for NetBSD).

Having spent ten minutes scribbling in xournal using my stylus, I can say
that this appears to work well. Diagonal lines are smooth even when drawn
fast. ws.c is looking much tidier too.

If the others are happy with the kernel portion of your diff, then I
reckon this is good to go in.

--
Best Regards
Edd Barrett

http://www.theunixzoo.co.uk

Reply | Threaded
Open this post in threaded view
|

Re: Stairstep mouse motion

Matthieu Herrb
In reply to this post by Alexander Shadchin
On Tue, Oct 29, 2013 at 11:07:29PM +0600, Alexandr Shadchin wrote:
>
> Update diff (add small hack for NetBSD).

Thanks. This (plus the kernel diff) is ok matthieu@.

>
> --
> Alexandr Shadchin
>
> Index: ws.c
> ===================================================================
> RCS file: /cvs/xenocara/driver/xf86-input-ws/src/ws.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 ws.c
> --- ws.c 20 Jul 2013 13:24:50 -0000 1.58
> +++ ws.c 29 Oct 2013 16:57:05 -0000
> @@ -428,13 +428,6 @@ wsDeviceOn(DeviceIntPtr pWS)
>   }
>   }
>   }
> - priv->buffer = XisbNew(pInfo->fd,
> -    sizeof(struct wscons_event) * NUMEVENTS);
> - if (priv->buffer == NULL) {
> - xf86IDrvMsg(pInfo, X_ERROR, "cannot alloc xisb buffer\n");
> - wsClose(pInfo);
> - return !Success;
> - }
>   xf86AddEnabledDevice(pInfo);
>   wsmbEmuOn(pInfo);
>   pWS->public.on = TRUE;
> @@ -462,170 +455,157 @@ wsDeviceOff(DeviceIntPtr pWS)
>   xf86RemoveEnabledDevice(pInfo);
>   wsClose(pInfo);
>   }
> - if (priv->buffer) {
> - XisbFree(priv->buffer);
> - priv->buffer = NULL;
> - }
>   pWS->public.on = FALSE;
>  }
>  
> -static void
> -wsReadInput(InputInfoPtr pInfo)
> +static Bool
> +wsReadEvent(InputInfoPtr pInfo, struct wscons_event *event)
>  {
> - WSDevicePtr priv = (WSDevicePtr)pInfo->private;
> - static struct wscons_event eventList[NUMEVENTS];
> - int n, c, dx, dy;
> - struct wscons_event *event = eventList;
> - unsigned char *pBuf;
> -
> - XisbBlockDuration(priv->buffer, -1);
> - pBuf = (unsigned char *)eventList;
> - n = 0;
> - while (n < sizeof(eventList) && (c = XisbRead(priv->buffer)) >= 0) {
> - pBuf[n++] = (unsigned char)c;
> + Bool rc = TRUE;
> + ssize_t len;
> +
> + len = read(pInfo->fd, event, sizeof(struct wscons_event));
> + if (len <= 0) {
> + if (errno != EAGAIN)
> + xf86IDrvMsg(pInfo, X_ERROR, "read error %s\n",
> +    strerror(errno));
> + rc = FALSE;
> + } else if (len != sizeof(struct wscons_event)) {
> + xf86IDrvMsg(pInfo, X_ERROR,
> +    "read error, invalid number of bytes\n");
> + rc = FALSE;
>   }
>  
> - if (n == 0)
> - return;
> + return rc;
> +}
>  
> - dx = dy = 0;
> - n /= sizeof(struct wscons_event);
> - while (n--) {
> - int buttons = priv->lastButtons;
> - int newdx = 0, newdy = 0, dz = 0, dw = 0, ax = 0, ay = 0;
> - int zbutton = 0, wbutton = 0;
> +static Bool
> +wsReadHwState(InputInfoPtr pInfo, wsHwState *hw)
> +{
> + WSDevicePtr priv = (WSDevicePtr)pInfo->private;
> + struct wscons_event event;
> +
> + bzero(hw, sizeof(wsHwState));
> +
> + hw->buttons = priv->lastButtons;
> + hw->ax = priv->old_ax;
> + hw->ay = priv->old_ay;
>  
> - switch (event->type) {
> + while (wsReadEvent(pInfo, &event)) {
> + switch (event.type) {
>   case WSCONS_EVENT_MOUSE_UP:
> - buttons &= ~(1 << event->value);
> - DBG(4, ErrorF("Button %d up %x\n", event->value,
> -    buttons));
> + hw->buttons &= ~(1 << event.value);
> + DBG(4, ErrorF("Button %d up %x\n", event.value,
> +    hw->buttons));
>   break;
>   case WSCONS_EVENT_MOUSE_DOWN:
> - buttons |= (1 << event->value);
> - DBG(4, ErrorF("Button %d down %x\n", event->value,
> -    buttons));
> + hw->buttons |= (1 << event.value);
> + DBG(4, ErrorF("Button %d down %x\n", event.value,
> +    hw->buttons));
>   break;
>   case WSCONS_EVENT_MOUSE_DELTA_X:
> - if (!dx)
> - dx = event->value;
> - else
> - newdx = event->value;
> - DBG(4, ErrorF("Relative X %d\n", event->value));
> + hw->dx = event.value;
> + DBG(4, ErrorF("Relative X %d\n", event.value));
>   break;
>   case WSCONS_EVENT_MOUSE_DELTA_Y:
> - if (!dy)
> - dy = -event->value;
> - else
> - newdy = -event->value;
> - DBG(4, ErrorF("Relative Y %d\n", event->value));
> + hw->dy = -event.value;
> + DBG(4, ErrorF("Relative Y %d\n", event.value));
> + break;
> + case WSCONS_EVENT_MOUSE_DELTA_Z:
> + hw->dz = event.value;
> + DBG(4, ErrorF("Relative Z %d\n", event.value));
> + break;
> + case WSCONS_EVENT_MOUSE_DELTA_W:
> + hw->dw = event.value;
> + DBG(4, ErrorF("Relative W %d\n", event.value));
>   break;
>   case WSCONS_EVENT_MOUSE_ABSOLUTE_X:
> - DBG(4, ErrorF("Absolute X %d\n", event->value));
> - if (event->value == 4095)
> - break;
> - ax = event->value;
> + hw->ax = event.value;
>   if (priv->inv_x)
> - ax = priv->max_x - ax + priv->min_x;
> + hw->ax = priv->max_x - hw->ax + priv->min_x;
> + DBG(4, ErrorF("Absolute X %d\n", event.value));
>   break;
>   case WSCONS_EVENT_MOUSE_ABSOLUTE_Y:
> - DBG(4, ErrorF("Absolute Y %d\n", event->value));
> - ay = event->value;
> + hw->ay = event.value;
>   if (priv->inv_y)
> - ay = priv->max_y - ay + priv->min_y;
> - break;
> - case WSCONS_EVENT_MOUSE_DELTA_Z:
> - DBG(4, ErrorF("Relative Z %d\n", event->value));
> - dz = event->value;
> + hw->ay = priv->max_y - hw->ay + priv->min_y;
> + DBG(4, ErrorF("Absolute Y %d\n", event.value));
>   break;
>   case WSCONS_EVENT_MOUSE_ABSOLUTE_Z:
> + case WSCONS_EVENT_MOUSE_ABSOLUTE_W:
>   /* ignore those */
> - ++event;
>   continue;
> - break;
> - case WSCONS_EVENT_MOUSE_DELTA_W:
> - DBG(4, ErrorF("Relative W %d\n", event->value));
> - dw = event->value;
> - break;
> + case WSCONS_EVENT_SYNC:
> + DBG(4, ErrorF("Sync\n"));
> + return TRUE;
>   default:
>   xf86IDrvMsg(pInfo, X_WARNING,
> -    "bad wsmouse event type=%d\n", event->type);
> - ++event;
> +    "bad wsmouse event type=%d\n", event.type);
>   continue;
>   }
> - ++event;
> +#ifdef __NetBSD__
> + /*
> + * XXX NetBSD reads only one event.
> + * WSCONS_EVENT_SYNC is not supported yet.
> + */
> + return TRUE;
> +#endif
> + }
>  
> - if ((newdx || newdy) || ((dx || dy) &&
> -    event->type != WSCONS_EVENT_MOUSE_DELTA_X &&
> -    event->type != WSCONS_EVENT_MOUSE_DELTA_Y)) {
> - int tmpx = dx, tmpy = dy;
> - dx = newdx;
> - dy = newdy;
> -
> - if (wsWheelEmuFilterMotion(pInfo, tmpx, tmpy))
> - continue;
> -
> - /* relative motion event */
> - DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
> -    tmpx, tmpy));
> - xf86PostMotionEvent(pInfo->dev, 0, 0, 2, tmpx, tmpy);
> - }
> - if (dz && priv->Z.negative != WS_NOMAP
> -    && priv->Z.positive != WS_NOMAP) {
> - zbutton = (dz < 0) ? priv->Z.negative :
> -    priv->Z.positive;
> - DBG(4, ErrorF("Z -> button %d\n", zbutton));
> - wsButtonClicks(pInfo, zbutton, abs(dz));
> - }
> - if (dw && priv->W.negative != WS_NOMAP
> -    && priv->W.positive != WS_NOMAP) {
> - wbutton = (dw < 0) ? priv->W.negative :
> -    priv->W.positive;
> - DBG(4, ErrorF("W -> button %d\n", wbutton));
> - wsButtonClicks(pInfo, wbutton, abs(dw));
> - }
> - if (priv->lastButtons != buttons) {
> - /* button event */
> - wsSendButtons(pInfo, buttons);
> - }
> - if (priv->swap_axes) {
> - int tmp;
> + return FALSE;
> +}
>  
> - tmp = ax;
> - ax = ay;
> - ay = tmp;
> - }
> - if (ax) {
> - int xdelta = ax - priv->old_ax;
> - priv->old_ax = ax;
> - if (wsWheelEmuFilterMotion(pInfo, xdelta, 0))
> - continue;
> -
> - /* absolute position event */
> - DBG(3, ErrorF("postMotionEvent X %d\n", ax));
> - xf86PostMotionEvent(pInfo->dev, 1, 0, 1, ax);
> - }
> - if (ay) {
> - int ydelta = ay - priv->old_ay;
> - priv->old_ay = ay;
> - if (wsWheelEmuFilterMotion(pInfo, 0, ydelta))
> - continue;
> -
> - /* absolute position event */
> - DBG(3, ErrorF("postMotionEvent y %d\n", ay));
> - xf86PostMotionEvent(pInfo->dev, 1, 1, 1, ay);
> - }
> +static void
> +wsReadInput(InputInfoPtr pInfo)
> +{
> + WSDevicePtr priv = (WSDevicePtr)pInfo->private;
> + wsHwState hw;
> +
> + if (!wsReadHwState(pInfo, &hw))
> + return;
> +
> + if ((hw.dx || hw.dy) && !wsWheelEmuFilterMotion(pInfo, hw.dx, hw.dy)) {
> + /* relative motion event */
> + DBG(3, ErrorF("postMotionEvent dX %d dY %d\n", hw.dx, hw.dy));
> + xf86PostMotionEvent(pInfo->dev, 0, 0, 2, hw.dx, hw.dy);
>   }
> - if (dx || dy) {
> - if (wsWheelEmuFilterMotion(pInfo, dx, dy))
> + if (hw.dz && priv->Z.negative != WS_NOMAP
> +    && priv->Z.positive != WS_NOMAP) {
> + int zbutton;
> + zbutton = (hw.dz < 0) ? priv->Z.negative : priv->Z.positive;
> + DBG(4, ErrorF("Z -> button %d (%d)\n", zbutton, abs(hw.dz)));
> + wsButtonClicks(pInfo, zbutton, abs(hw.dz));
> + }
> + if (hw.dw && priv->W.negative != WS_NOMAP
> +    && priv->W.positive != WS_NOMAP) {
> + int wbutton;
> + wbutton = (hw.dw < 0) ? priv->W.negative : priv->W.positive;
> + DBG(4, ErrorF("W -> button %d (%d)\n", wbutton, abs(hw.dw)));
> + wsButtonClicks(pInfo, wbutton, abs(hw.dw));
> + }
> + if (priv->lastButtons != hw.buttons) {
> + /* button event */
> + wsSendButtons(pInfo, hw.buttons);
> + }
> + if (priv->swap_axes) {
> + int tmp;
> +
> + tmp = hw.ax;
> + hw.ax = hw.ay;
> + hw.ay = tmp;
> + }
> + if ((hw.ax != priv->old_ax) || (hw.ay != priv->old_ay)) {
> + int xdelta = hw.ax - priv->old_ax;
> + int ydelta = hw.ay - priv->old_ay;
> + priv->old_ax = hw.ax;
> + priv->old_ay = hw.ay;
> + if (wsWheelEmuFilterMotion(pInfo, xdelta, ydelta))
>   return;
>  
> - /* relative motion event */
> - DBG(3, ErrorF("postMotionEvent dX %d dY %d\n",
> -    dx, dy));
> - xf86PostMotionEvent(pInfo->dev, 0, 0, 2, dx, dy);
> + /* absolute position event */
> + DBG(3, ErrorF("postMotionEvent X %d Y %d\n", hw.ax, hw.ay));
> + xf86PostMotionEvent(pInfo->dev, 1, 0, 2, hw.ax, hw.ay);
>   }
> - return;
>  }
>  
>  static void
> Index: ws.h
> ===================================================================
> RCS file: /cvs/xenocara/driver/xf86-input-ws/src/ws.h,v
> retrieving revision 1.12
> diff -u -p -r1.12 ws.h
> --- ws.h 12 Jun 2012 17:44:56 -0000 1.12
> +++ ws.h 29 Oct 2013 16:57:05 -0000
> @@ -29,7 +29,6 @@ extern int ws_debug_level;
>  #define NAXES 2 /* X and Y axes only */
>  #define NBUTTONS 32 /* max theoretical buttons */
>  #define DFLTBUTTONS 3 /* default number of buttons */
> -#define NUMEVENTS 16 /* max # of ws events to read at once */
>  
>  #define WS_NOMAP 0
>  
> @@ -40,6 +39,12 @@ typedef struct {
>   int traveled_distance;
>  } WheelAxis, *WheelAxisPtr;
>  
> +typedef struct {
> + unsigned int buttons;
> + int dx, dy, dz, dw;
> + int ax, ay;
> +} wsHwState;
> +
>  typedef struct WSDevice {
>   char *devName; /* device name */
>   int type; /* ws device type */
> @@ -50,7 +55,6 @@ typedef struct WSDevice {
>   int raw;
>   int inv_x, inv_y;
>   int screen_no;
> - pointer buffer;
>   WheelAxis Z;
>   WheelAxis W;
>   struct wsmouse_calibcoords coords; /* mirror of the kernel values */

--
Matthieu Herrb