[PATCH 1/5] winebus.sys: Report WINEBUS\WINE_COMP_HID compatible id.

[Rémi Bernon]

On 8/19/21 5:29 AM, Zebediah Figura (she/her) wrote:
> On 8/18/21 5:04 PM, Rémi Bernon wrote:
>> On 8/18/21 11:51 PM, Zebediah Figura (she/her) wrote:
>>>
>>> Ah, I see what I was missing. So it's kind of a poor man's filter DO, I
>>> guess. That is, it effectively inserts itself into the device stack
>>> between winebus and winehid, and exposes the HID interface from winebus
>>> directly using an internal GUID, while reporting a different HID
>>> descriptor (and report data, etc.) to winehid?
>>>
>>
>> Yes. I quickly looked at filter drivers but didn't find anything
>> explicit and it looked like we don't have the necessary bricks to make
>> it work. And I really don't think it's worth the trouble for something
>> as simple as this driver.
>>
> 
> Filter drivers wouldn't help anyway, because of the mentioned problem 
> with IRP_MJ_CREATE. As far as I know, if we could do it with a filter 
> driver, we could do it with winehid.
> 
>>> I must still be missing something, though, because if that's the case I
>>> don't see why you would need to touch hidclass at all.
>>>
>>> I guess the only reason we can't do this all from winehid.sys is because
>>> hidclass doesn't let anyone open the device directly, right?
>>>
>>
>> Well I don't know how to tell hidclass.sys not register the device on
>> the HID GUID, and register it instead on another GUID without changing
>> hidclass.sys.
>>
>> And yes, opening any HID device goes through hidclass.sys and we really
>> want it to be on top and handle all the ugly HID details (and it also
>> guarantees that we will only ever have a single read request at a time,
>> which will be handy).
>>
> 
> Mmh, I see the problem now. Hmm.
> 
> I guess we really do want both the real and xinput devices to go through 
> winehid + hidclass.
> 
> It should come as no surprise that the hidclass hack isn't great. But I 
> can't particularly hate it, because this solution is in general about as 
> close as we can get to native architecture.
> 

I agree, I sadly couldn't find another way. I guess it's the drawback of 
using HID to communicate with the xinput internal device as it should 
normally not be that way.

> I'm almost tempted to propose shoving large parts of hidclass into 
> xinput and/or xinput.sys (making use of that new hidparse, probably—by 
> the way, could that be a static library instead of using PARENTSRC?) but 
> it's probably too much work to be worthwhile.
> 

FWIW native hidparse.sys really exists, I don't know how much it matters 
to stay close to native in the world of drivers though.

I don't really like the idea of making xinput having to re-implement 
similar logic as hidclass.sys, even if most of the code could be 
re-imported. The way it's done here make it possible to keep the driver 
code very simple and I don't think we want it to become complicated.

> How bad would it be if we exposed the real device using 
> GUID_DEVINTERFACE_HID anyway? Would it be worse than duplicate devices 
> in some games? Not that that's particularly desirable to begin with, 
> granted.
> 

Yeah, I don't really now but I suspect it's not a good idea. Also we 
have to have a way to differentiate xinput-compatible HID devices from 
xinput-incompatible gamepads in xinput itself (although it could very 
well be done with another kind of suffix).

> Otherwise I'll probably start reviewing the actual contents of this 
> patch series instead of trying to wrap my head around what it's actually 
> doing ;-)
> 

Thanks!
-- 
Rémi Bernon <rbernon at codeweavers.com>