Patch for dinput keyboard

[arjen at blehq.org]

On Sun, 19 May 2002, Ove Kaaven wrote:

>
> On Sat, 18 May 2002 arjen at blehq.org 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.