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