winejoystick.dev: Add Graphics Driver connection for winejoystick.drv

[Aric Stewart]

Hi Ken,

Thanks for the comments.

On 4/12/15 9:27 PM, Ken Thomases wrote:
> On Apr 10, 2015, at 1:08 PM, Charles Davis <cdavis5x at gmail.com> wrote:
> 
>> I do not agree with this particular direction. You do realize that some people on Mac OS still use the X11 driver (e.g. to get virtual desktop support), right? With these patches, these people suddenly won’t have any joystick support at all.
>>
>> Don’t get me wrong, I still think putting all the joystick code in one place is the right thing to do. I just don’t think that one place should be the graphics driver.
> 
> I agree with Chip on this.
> 

I feel like the joystick code should be collected somewhere. Having platform specific code scattered about the code base is a pain in the butt. So maybe I push back on Alexandre and see if we should define this as a joystick driver.  The Mac driver can happen to be the default joystick driver for the mac? 


> Let's look at the other side, the non-Mac side.  Besides Mac joystick/gamepad support, there's also support for Linux.  Is that to be moved into the X11 driver?  As it currently stands, it doesn't use X11.  Furthermore, the X11 driver is not the Linux driver in the same way that the Mac driver is, well, the _Mac_ driver.  That is, the X11 driver is not for a specific platform, so it doesn't make much sense to move the Linux joystick support to it.
> 
> 
> Besides that general concern, there are problems with the patches.  For example, in 0013-dinput-Add-Graphic-driver-connection-for-dinput.txt, the values array is allocated with one entry per axis, POV, and button.  But POVs store 2 values (x and y).  So, if there are any POVs, you'll overrun the array.
> 
> In 0016-winemac.drv-Add-Joystick-driver-to-the-mac-driver.txt, pov_idx is not really used.  It's initialized and incremented for each POV enumerated, but nothing ever makes use of the value.
> 
> Those happened to jumped out at me.  I haven't done a thorough review.
> 

It is a lot of code that I have worked through, I tried to debug and test extensively but I expected there would be some flaws. Thank you for finding these. 



> I'm not sure why the force feedback interfaces were added in a separate patch.
> 

This was because of the size that the first patch was growing to. It seemed like a logical place to divide the work to not make it one monolithic patch.

> 
> I worry about the design of the driver interface for this joystick/gamepad support.  In particular, it sets in stone the polling nature of the current implementation.  DInput supports buffered data and "spontaneous" notification via event rather than polling.  (Wine's dinput does event notification, but the app has to poll to get the events.)  The HID API of OS X supports similar features.  The current implementation doesn't take advantage of this similarity, but it arguably should.  For example, currently, button press-and-release sequences that occur between polls are lost.  If the dinput OS X joystick support were changed to support these capabilities, it has all the flexibility it needs because it controls the whole device vtable.  The interface you've designed for the graphics driver joystick support enforces just one approach.  It's not clear how the graphics driver would support spontaneous event notification, for example, because that would probably require a call back to di
nput.
> 

I designed this driver interface to match the current code. Which was all poll based as you pointed out.  All your flaws with the poll base code exist currently as well. If we want to move towards a push base possibility. I do not think that is hard. It is just a matter of adding some interfaces into the joystick driver that would support a push endpoint and then anything that wanted push data would have to implement defined callbacks that receive the data from the driver when it is available. 

I do not see that as an impossible or even difficult extension to the current work I did for the poll based reporting. I would not see the current approach as enforcing this poll based reporting as the only possible way for the future.

-aric