On Sun, 19 May 2002, Ove Kaaven wrote:
>
> On Sat, 18 May 2002 [EMAIL PROTECTED] wrote:
>
> > This 'beautiful' patch I wrote for fixing dinput was silently ignored.
> > Can someone comment about it?
>
> The x11drv parts are unnecessary. It's done differently (more
> Alexandre-style) in WineX. Since there's no particular need to hold this
> piece back whether or not the code trade Gav wants to do is successful
> (especially since you're working on it in any case), I may as well use it
> to help you get this functionality into Wine. Here's the code merged
> against ReWind, licensed under the X11 license. You may have to merge it
> again against the current Wine code yourself, since some other DirectInput
> patches may not have been applied to ReWind. (Of course, change this in
> other ways as well if you feel like it.)
>
Thank you, I guess. I don't want to get involved in any politics. I don't
really know anything about other trees.
I think the hook is agood idea.
I think the best thing to do now is to get best out of the two different
implementations. I have some comments about your code, and I can improve
mine with yours. I'll make a new patch from these two, and send it later.
If anybody has any other comments, I would like to hear them.
> + /* SysKeyboardAImpl */
> + BYTE keystate[256];
There is not a real reason to get a keystate per keyboard, as there is
only one keyboard. I made this global. Is that OK?
> static HRESULT WINAPI SysKeyboardAImpl_SetProperty(
> LPDIRECTINPUTDEVICE2A iface,REFGUID rguid,LPCDIPROPHEADER ph
> )
> @@ -115,13 +169,19 @@
>
> TRACE("(this=%p,%s,%p)\n",This,debugstr_guid(rguid),ph);
> TRACE("(size=%ld,headersize=%ld,obj=%ld,how=%ld\n",
> - ph->dwSize,ph->dwHeaderSize,ph->dwObj,ph->dwHow);
> + ph->dwSize,ph->dwHeaderSize,ph->dwObj,ph->dwHow);
> if (!HIWORD(rguid)) {
> switch ((DWORD)rguid) {
> case (DWORD) DIPROP_BUFFERSIZE: {
> - LPCDIPROPDWORD pd = (LPCDIPROPDWORD)ph;
> -
> - TRACE("(buffersize=%ld)\n",pd->dwData);
> + LPCDIPROPDWORD pd = (LPCDIPROPDWORD)ph;
> +
> + TRACE("buffersize = %ld\n",pd->dwData);
> +
> + This->data_queue =
>(LPDIDEVICEOBJECTDATA)HeapAlloc(GetProcessHeap(),0,
> + pd->dwData
>* sizeof(DIDEVICEOBJECTDATA));
How do you know the data_queue is free (NULL)? My patch made sure that:
!Acquired => (data_queue == NULL)
And setproperty can only be invoked when !Acquired.
> static HRESULT WINAPI SysKeyboardAImpl_GetDeviceState(
> LPDIRECTINPUTDEVICE2A iface,DWORD len,LPVOID ptr
> )
> {
> - DWORD i;
> + ICOM_THIS(SysKeyboardAImpl,iface);
> +
> + if (!This->acquired)
> + return DIERR_NOTACQUIRED;
I have this game that does not acquire the device. Guess what? It works
with windows.
>
> - memset( ptr, 0, len );
> - if (len != 256)
> - {
> - WARN("whoops, got len %ld?\n", len);
> - return DI_OK;
> - }
> - for (i = 0; i < 0x80; i++)
> - {
> - WORD vkey = MapVirtualKeyA( i, 1 );
> - if (vkey && (GetAsyncKeyState( vkey ) & 0x8000))
> - {
> - ((LPBYTE)ptr)[i] = 0x80;
> - ((LPBYTE)ptr)[i | 0x80] = 0x80;
> - }
> - }
> - return DI_OK;
> + memset( ptr, 0, len );
> + if (len != 256)
> + {
> + WARN("whoops, got len %ld?\n", len);
> + return DI_OK;
DI_OK?
> + }
> + memcpy(ptr, This->keystate, len);
> + return DI_OK;
> }
>
> static HRESULT WINAPI SysKeyboardAImpl_GetDeviceData(
> @@ -162,34 +258,53 @@
> )
> {
> ICOM_THIS(SysKeyboardAImpl,iface);
> - int i, n;
> + DWORD len, nqtail;
> +
> + EnterCriticalSection(&(This->crit));
> + TRACE("(%p)->(dods=%ld,entries=%ld,fl=0x%08lx)\n",This,dodsize,*entries,flags);
>
> - TRACE("(this=%p,%ld,%p,%p(%ld)),0x%08lx)\n",
> - This,dodsize,dod,entries,entries?*entries:0,flags);
> + len = ((This->queue_head < This->queue_tail) ? This->queue_len : 0)
> + + (This->queue_head - This->queue_tail);
> + if (len > *entries) len = *entries;
> +
> + if (dod == NULL) {
> + if (len)
> + TRACE("Application discarding %ld event(s).\n", len);
> +
> + *entries = len;
> + nqtail = This->queue_tail + len;
> + while (nqtail >= This->queue_len) nqtail -= This->queue_len;
> + } else {
> + if (dodsize < sizeof(DIDEVICEOBJECTDATA)) {
> + ERR("Wrong structure size !\n");
> + LeaveCriticalSection(&(This->crit));
> + return DIERR_INVALIDPARAM;
> + }
> +
> + if (len)
> + TRACE("Application retrieving %ld event(s).\n", len);
> +
> + *entries = 0;
> + nqtail = This->queue_tail;
> + while (len) {
> + DWORD span = ((This->queue_head < nqtail) ? This->queue_len :
>This->queue_head)
If dodsize > 16 then span = 1, but I don't really get the algorithm. Do
you know, that most of the time there is not more than one entry in the
buffer. I once read a peace about UNIX and algorithms and N being small...
Well, never mind.
> + - nqtail;
> + if (span > len) span = len;
> + /* Copy the buffered data into the application queue */
> + memcpy(dod + *entries, This->data_queue + nqtail, span * dodsize);
> + /* Advance position */
> + nqtail += span;
> + if (nqtail >= This->queue_len) nqtail -= This->queue_len;
> + *entries += span;
> + len -= span;
> + }
> + }
> + if (!(flags & DIGDD_PEEK))
> + This->queue_tail = nqtail;
>
> + LeaveCriticalSection(&(This->crit));
> + return DI_OK;
> }
I think my implementation is cleaner.
>
> static HRESULT WINAPI SysKeyboardAImpl_Acquire(LPDIRECTINPUTDEVICE2A iface)
> @@ -199,10 +314,27 @@
> TRACE("(this=%p)\n",This);
>
> if (This->acquired == 0) {
> - This->acquired = 1;
> + DWORD i;
> +
> + /* Store (in a global variable) the current lock */
> + current_lock = (IDirectInputDevice2A*)This;
> +
> + /* Install our keyboard hook */
> + This->hook = SetWindowsHookExW( WH_KEYBOARD_LL, dinput_keyboard_hook, 0, 0
>);
> +
> + /* Read current keyboard state */
> + memset(&This->keystate, 0, 256);
> + for (i = 0; i < 0x100; i++)
> + {
> + WORD vkey = MapVirtualKeyA( i, 1 ); /* FIXME: use map mode 3 when
>implemented */
> + if (vkey && (GetAsyncKeyState( vkey ) & 0x8000))
This does not work (see earlier mails). but it isn't needed when keystate
is global.
> + This->keystate[i] = 0x80;
> + }
> +
> + This->acquired = 1;
> + return DI_OK;
> }
> -
> - return DI_OK;
> + return S_FALSE;
> }
>
Could you send me the file instead of the patch, as I don't really know
what to do with this patch.
To get involved in politics anyway: you can have the changes I make to
this patch.
