[RFC PATCH] dinput: Introduce new HID joystick backend.

[Zebediah Figura (she/her)]

On 6/17/21 11:47 AM, Rémi Bernon wrote:
> On 6/17/21 6:40 PM, Zebediah Figura (she/her) wrote:
>> On 6/1/21 4:09 AM, Rémi Bernon wrote:
>>> Sending this as RFC because I'm not completely sure how to proceed (and
>>> also because ideally it would depend on 206872 which is not merged yet).
>>>
>>> The problem is that this new backend will fail most dinput(8) tests
>>> until it's more properly implemented, although the testbot doesn't have
>>> HID devices to test with.
>>>
>>> It could also cause issues if the new backend is enumerated and somehow
>>> is selected by default for some reason.
>>>
>>> Either we don't really care, or the new backend could be conditionally
>>> enabled with a configure flag?
>>
>> For whatever it's worth, this strikes me as a good idea.
>>
>> Probably what should be done is to deprioritize or even explicitly
>> disable this backend in the registry until it's ready to work, at which
>> point we can make it the default, keeping the old backends around until
>> all the bugs get ironed out. I don't know whether it's worth completely
>> disabling it just to make the tests pass for the TestBot and Alexandre;
>> I guess I'll leave that one to them.
> 
> I don't think there's a registry entry for that? The backends are just
> enumerated in order. It could be added I guess but I'm not even sure
> it's worth it. I think the way it usually works is that each backend
> lists their device, creating physically duplicated devices but with
> different features (like the linux and linuxinput joysticks).

Right, we'd have to add one.

>> Of course, introducing a switch like that is a good way to ensure that
>> concerned users always turn it off, and that the new backend never
>> really gets tested, but it would be unfair to offer no way to revert to
>> the old behaviour...
>>
> 
> I don't even think the TestBot will notice, as it doesn't have any
> joysticks.

Well, I'm thinking of users here. I think it's inevitable that the new 
backend will expose a bunch of bugs, in various parts of the HID stack, 
and we'll want a way to confirm that a bug is due to the new HID 
backend, and maybe also a way to let users work around such bugs.

Ideally once the HID dinput backend is fully implemented it should be 
made the default (for applications which e.g. default to the first 
joystick available, or for those which don't even offer the user a 
choice). Otherwise it won't get tested at all.

>> (Side note: looking at this patch shows me just how ugly the
>> IDirectInputDevice8WVtbl implementation is. If it's really worth sharing
>> all that code, I'd advocate for using a struct dinput_device_ops
>> instead...)
>>
> 
> Yeah although that would require cleaning up all the other joystick
> backend code, which I understand will ultimately be removed so it's a
> bit of a waste of time. Finishing the cleanup after the HID backend is
> complete is probably simpler.
> 

Yeah, agreed.