[PATCH v10 2/9] server: Move mouse raw-input message faking from user32 to wineserver.

[Zebediah Figura]

On 8/5/19 1:32 PM, Derek Lesho wrote:
> On Mon, Aug 5, 2019 at 2:19 PM Zebediah Figura <zfigura at codeweavers.com> wrote:
>>
>> On 8/5/19 12:22 PM, Derek Lesho wrote:
>>> What we do now is send a win32-defined INPUT structure to
>>> __wine_send_input and send_hardware_message.  If we want to send raw
>>> data alongside it we'll have probably need to send in a RAWINPUT
>>> structure as well as an INPUT structure to these functions.  Most
>>> invocations of __wine_send_input and send_hardware_message aren't
>>> sending raw-input data at all, but since C doesn't have optional
>>> parameters we'd have to refactor all invocations of it.
>>
>> This doesn't seem particularly hard. You could even have winemac and
>> wineandroid pass NULL, and have user32 fall back to copying from the
>> INPUT structure in that case.
>>
>> For that matter, if the only thing that actually matters is the cursor
>> position, you don't even have to pass in a whole RAWINPUT structure, but
>> instead just extra x/y parameters, or a POINT structure.
> Take a look at patch 8, we are also sending raw mouse wheel and mouse
> button data, so we might as well use the RAWINPUT structure in case
> there are any future quirks with say rawinput keyboard messages.

Sure, I didn't bother looking at the whole patch set. The same point 
applies.

> 
>>
>> As yet another alternative, we could just pass in a hw_input_t. If it
>> were my code that's probably what I'd do.
>>
>>> This also doesn't solve your primary concern, which is the static
>>> variable to indicate whether we want to emulate raw-input mouse
>>> movement.  Yes we could duplicate the raw-input emulation code into
>>> all the display drivers for the mouse now and the keyboard in the
>>> future, but the current solution completely avoids this, and allows
>>> display drivers that have raw-input capabilities to seamlessly send
>>> what they want.
>>>
>>
>> By the "emulate_raw_mouse" variable I metonymically mean all the logic
>> introduced by this patch series, but essentially, yes. I disagree here
>> with this concern: I don't think deduplication should always be the
>> primary goal, and in this case I think it makes the logic more
>> complicated than it needs to be.
> This is a matter of preference, but I think that adding 3 lines of
> code to disable emulation when we get native events is not overly
> complicated.

It's not just a matter of adding 3 lines of code (or as many lines of 
code as your patch 5/9 in fact adds). You also have to move the 
"emulation" to the server, and you have to have some way of signaling to 
the server that raw input is supported, as your patch 3/9 does.

Not to mention that using a static variable in patch 5/9 isn't correct. 
You can't depend on all clients equally supporting raw input. For that 
matter, I'm not sure you should depend on one client unconditionally 
supporting raw input.

In general I'm inclined to think that most of this logic should stay out 
of wineserver. It's the USER driver's responsibility to send the most 
accurate input that it can. If that means, for now, that some drivers 
send the same values for normal and raw input, that's not the end of the 
world. Again, not that line count should be the primary concern, but I 
suspect the amount of code you'd add this way would even be less than 
the amount of code this current iteration of the patch set changes.

>>
>> Besides, if ultimately the goal is to send raw input from all USER
>> drivers (and I see no reason why this should not be the case), then
>> there's barely any point trying to make "emulation" simpler.
>>
>> (By the way, it'd be appreciated if you could bottom-post in replies.)
> Sorry, I use G-Mail which top-posts by default.
> 

Yes, you may have to change your mail client's defaults if they aren't 
set to sensible values.