Re: joystick GetDeviceData patch

Rob Crittenden <[email protected]>
Newsgroups gmane.comp.emulators.winex.devel
Message-ID <[email protected]> 
On Mon, 29 Mar 2004, Ove Kaaven wrote:
> fre, 26.03.2004 kl. 04.44 skrev Rob Crittenden:
> 
> Does BF1942 need this overflow feature? If it's needed, can't you also
> just add this feature to the mouse code as well, so it isn't an unused
> state there?

Here is a patch with mouse overflow detection too. It also catches the
case where buffering hasn't been setup and GetDeviceData() is called anyway.
I don't think either of these will cause any problems.

BF1942 uses GetDeviceData() for mice too and I didn't detect any issues
with the patch. I wasn't able to overflow the buffer though. It has a size
of 20 and the most events I was able to generate was a tantilizing 19. This
was through extremely spastic movement and clicking. I tried and tried and
was never able to cause an overflow. One may argue whether this is a good thing
or not. In the case of an overflow state data is still returned to the game
but the status is DI_BUFFEROVERFLOW indicating that some state data was lost.

As to whether it's needed, well, it depends on what the goal is. It is a
fairly straightforward patch that brings the joystick code I *think* fully
up-to-spec. The mouse code is pretty darn close too though I admit I didn't
examine it in detail related to compliance. There are enough other mice
issues that I didn't want to do a full rip-out of the otherwise working
mouse code for some minor compliance reason that one doesn't see in the
"real world." No point in being pedantic.

rob
joystick.diff (text/plain, 8.5 KB) 
Index: device.c
===================================================================
RCS file: /cvsroot/winex/dlls/dinput/device.c,v
retrieving revision 1.15
diff -u -r1.15 device.c
--- device.c	21 Dec 2003 10:57:34 -0000	1.15
+++ device.c	30 Mar 2004 03:57:44 -0000
@@ -107,20 +107,20 @@
       if (df->dt[i].offset_in >= 0) {
 	switch (df->dt[i].size) {
 	case 1:
-	  TRACE("Copying (c) to %d from %d (value %d)\n",
-		df->dt[i].offset_out, df->dt[i].offset_in, *((char *) (in_c + df->dt[i].offset_in)));
+//	  TRACE("Copying (c) to %d from %d (value %d)\n",
+//		df->dt[i].offset_out, df->dt[i].offset_in, *((char *) (in_c + df->dt[i].offset_in)));
 	  *((char *) (out_c + df->dt[i].offset_out)) = *((char *) (in_c + df->dt[i].offset_in));
 	  break;
 
 	case 2:
-	  TRACE("Copying (s) to %d from %d (value %d)\n",
-		df->dt[i].offset_out, df->dt[i].offset_in, *((short *) (in_c + df->dt[i].offset_in)));
+//	  TRACE("Copying (s) to %d from %d (value %d)\n",
+//		df->dt[i].offset_out, df->dt[i].offset_in, *((short *) (in_c + df->dt[i].offset_in)));
 	  *((short *) (out_c + df->dt[i].offset_out)) = *((short *) (in_c + df->dt[i].offset_in));
 	  break;
 
 	case 4:
-	  TRACE("Copying (i) to %d from %d (value %d)\n",
-		df->dt[i].offset_out, df->dt[i].offset_in, *((int *) (in_c + df->dt[i].offset_in)));
+//	  TRACE("Copying (i) to %d from %d (value %d)\n",
+//		df->dt[i].offset_out, df->dt[i].offset_in, *((int *) (in_c + df->dt[i].offset_in)));
 	  *((int *) (out_c + df->dt[i].offset_out)) = *((int *) (in_c + df->dt[i].offset_in));
 	  break;
 
@@ -130,20 +130,20 @@
       } else {
 	switch (df->dt[i].size) {
 	case 1:
-	  TRACE("Copying (c) to %d default value %d\n",
-		df->dt[i].offset_out, df->dt[i].value);
+//	  TRACE("Copying (c) to %d default value %d\n",
+//		df->dt[i].offset_out, df->dt[i].value);
 	  *((char *) (out_c + df->dt[i].offset_out)) = (char) df->dt[i].value;
 	  break;
 
 	case 2:
-	  TRACE("Copying (s) to %d default value %d\n",
-		df->dt[i].offset_out, df->dt[i].value);
+//	  TRACE("Copying (s) to %d default value %d\n",
+//		df->dt[i].offset_out, df->dt[i].value);
 	  *((short *) (out_c + df->dt[i].offset_out)) = (short) df->dt[i].value;
 	  break;
 
 	case 4:
-	  TRACE("Copying (i) to %d default value %d\n",
-		df->dt[i].offset_out, df->dt[i].value);
+//	  TRACE("Copying (i) to %d default value %d\n",
+//		df->dt[i].offset_out, df->dt[i].value);
 	  *((int *) (out_c + df->dt[i].offset_out)) = (int) df->dt[i].value;
 	  break;
 
Index: device_private.h
===================================================================
RCS file: /cvsroot/winex/dlls/dinput/device_private.h,v
retrieving revision 1.11
diff -u -r1.11 device_private.h
--- device_private.h	15 Nov 2003 01:15:01 -0000	1.11
+++ device_private.h	30 Mar 2004 03:57:45 -0000
@@ -58,6 +58,8 @@
       This->data_queue[This->queue_head].dwSequence = seq;			\
       This->queue_head = nq;							\
     }										\
+    else									\
+      This->overflow = TRUE;							\
   }										\
 }
 
Index: joystick/main.c
===================================================================
RCS file: /cvsroot/winex/dlls/dinput/joystick/main.c,v
retrieving revision 1.6
diff -u -r1.6 main.c
--- joystick/main.c	21 Dec 2003 04:13:30 -0000	1.6
+++ joystick/main.c	30 Mar 2004 03:57:48 -0000
@@ -83,8 +83,10 @@
 	DataFormat*			wine_df;
 	ObjProps*			props;
         HANDLE				hEvent;
+        CRITICAL_SECTION                crit;
         LPDIDEVICEOBJECTDATA 		data_queue;
         int				queue_head, queue_tail, queue_len;
+        BOOL				overflow;
 
 	/* effects */
 	DWORD				dwAutoCenter;
@@ -210,6 +212,10 @@
       newDevice->props[u].lSaturation = 0; /* 0% */
     }
   }
+  newDevice->queue_len = -1; /* unbuffered */
+  newDevice->overflow = FALSE;
+
+  InitializeCriticalSection(&(newDevice->crit));
 
   newDevice->dwGain = 10000;
   newDevice->dwAutoCenter = 0;
@@ -292,6 +298,8 @@
   if (This->data_queue != NULL)
     HeapFree(GetProcessHeap(),0,This->data_queue);
 
+  DeleteCriticalSection(&(This->crit));
+
   /* Free the DataFormat */
   HeapFree(GetProcessHeap(), 0, This->offset_array);
   HeapFree(GetProcessHeap(), 0, This->buffer);
@@ -448,21 +456,57 @@
 					      DWORD flags
 ) {
   ICOM_THIS(JoystickAImpl,iface);
+  DWORD len, count, nqtail;
+  int ret = DI_OK;
 
-  FIXME("(%p)->(dods=%ld,entries=%ld,fl=0x%08lx),STUB!\n",This,dodsize,*entries,flags);
+  TRACE("(%p)->(dods=%ld,entries=%ld,fl=0x%08lx)\n",This,dodsize,*entries,flags);
 
   if (!This->acquired)
     return DIERR_NOTACQUIRED;
 
-  joy_polldev(This, FALSE, TRUE);
-  if (flags & DIGDD_PEEK)
-    FIXME("DIGDD_PEEK\n");
-  *entries = 0;
+  if (This->queue_len == -1)
+    return DIERR_NOTBUFFERED;
+
+  if (dodsize < sizeof(DIDEVICEOBJECTDATA_DX3))
+    return DIERR_INVALIDPARAM;
+
+  EnterCriticalSection(&(This->crit));
 
-  if (dod == NULL) {
-  } else {
+  len = ((This->queue_head < This->queue_tail) ? This->queue_len : 0)
+      + (This->queue_head - This->queue_tail);
+  if (len > *entries) len = *entries;
+
+  count = 0;
+  nqtail = This->queue_tail;
+  while ((count < *entries || *entries == INFINITE) && count < len) {
+    if (dod != NULL) {
+      LPDIDEVICEOBJECTDATA pd = (LPDIDEVICEOBJECTDATA)((BYTE *)dod + dodsize * count);
+      pd->dwOfs       = This->data_queue[nqtail].dwOfs;
+      pd->dwData      = This->data_queue[nqtail].dwData;
+      pd->dwTimeStamp = This->data_queue[nqtail].dwTimeStamp;
+      pd->dwSequence  = This->data_queue[nqtail].dwSequence;
+    }
+    nqtail++;
+    if (nqtail >= This->queue_len) nqtail -= This->queue_len;
+    count++;
   }
-  return 0;
+
+  if (This->overflow) {
+    ret = DI_BUFFEROVERFLOW;
+    This->overflow = FALSE;
+  }
+
+  if (!(flags & DIGDD_PEEK)) {
+    This->queue_tail = nqtail;
+  }
+
+  TRACE("Application %s %ld event(s). %s\n", *entries == INFINITE ? "flushing" : "retrieving", count, ret == DI_OK ? "" : "BUFFEROVERFLOW");
+
+  *entries = count;
+
+  LeaveCriticalSection(&(This->crit));
+
+  return ret;
 }
 
 int find_object(JoystickAImpl *This, DWORD dwHow, DWORD dwObj)
@@ -519,7 +563,13 @@
     case (DWORD) DIPROP_BUFFERSIZE: {
       LPCDIPROPDWORD	pd = (LPCDIPROPDWORD)ph;
 
-      FIXME("buffersize = %ld\n",pd->dwData);
+      TRACE("buffersize(%ld)\n", pd->dwData);
+     
+      This->data_queue = (LPDIDEVICEOBJECTDATA)HeapAlloc(GetProcessHeap(),0,
+                                                          pd->dwData * sizeof(DIDEVICEOBJECTDATA));
+      This->queue_head = 0;
+      This->queue_tail = 0;
+      This->queue_len = pd->dwData;
       break;
     }
     case (DWORD)DIPROP_RANGE: {
Index: mouse/main.c
===================================================================
RCS file: /cvsroot/winex/dlls/dinput/mouse/main.c,v
retrieving revision 1.46.2.2
diff -u -r1.46.2.2 main.c
--- mouse/main.c	2 Mar 2004 17:21:18 -0000	1.46.2.2
+++ mouse/main.c	30 Mar 2004 03:57:51 -0000
@@ -136,6 +136,7 @@
         DWORD				win_centerX, win_centerY;
         LPDIDEVICEOBJECTDATA 		data_queue;
         int				queue_head, queue_tail, queue_len;
+        BOOL				overflow;
 	/* warping: whether we need to move mouse back to middle once we
 	 * reach window borders (for e.g. shooters, "surface movement" games) */
         WARP_STATUS		        need_warp;
@@ -222,6 +223,9 @@
     newDevice->win = GetDesktopWindow();
     newDevice->dwCoopLevel = DISCL_NONEXCLUSIVE | DISCL_BACKGROUND;
 
+    newDevice->queue_len = -1; /* unbuffered */
+    newDevice->overflow = FALSE;
+
     return newDevice;
 }
 
@@ -806,6 +810,7 @@
 ) {
   ICOM_THIS(SysMouseAImpl,iface);
   DWORD len, count, nqtail;
+  int ret = DI_OK;
 
   TRACE("(%p)->(dods=%ld,entries=%ld,fl=0x%08lx)\n",This,dodsize,*entries,flags);
 
@@ -814,6 +819,10 @@
       return DIERR_NOTACQUIRED;
   }
 
+  if (This->queue_len == -1) {
+      return DIERR_NOTBUFFERED;
+  }
+
   EnterCriticalSection(&(This->crit));
 
   len = ((This->queue_head < This->queue_tail) ? This->queue_len : 0)
@@ -851,8 +860,14 @@
     }
     *entries = count;
   }
-  if (!(flags & DIGDD_PEEK))
+  if (This->overflow) {
+    ret = DI_BUFFEROVERFLOW;
+    This->overflow = FALSE;
+  }
+
+  if (!(flags & DIGDD_PEEK)) {
     This->queue_tail = nqtail;
+  }
 
   LeaveCriticalSection(&(This->crit));
 
@@ -862,7 +877,7 @@
     dinput_window_check(This);
     dinput_mouse_warp(This, FALSE);
   }
-  return 0;
+  return ret;
 }
 
 /******************************************************************************
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.