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

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 */

Reply via email to