Re: [PATCH resend 1/9] server: Add send_hardware_message flags for rawinput translation.

[Zebediah Figura]

Hello Rémi,

On 11/4/19 6:17 AM, Rémi Bernon wrote:
> Signed-off-by: Rémi Bernon
>
> server/protocol.def |  2 ++
>   server/queue.c      | 20 ++++++++++++++------
>   2 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/server/protocol.def b/server/protocol.def
> index 6af0ae0cff8..ab3af90545b 100644
> --- a/server/protocol.def
> +++ b/server/protocol.def
> @@ -2315,6 +2315,8 @@ enum message_type
>       VARARG(keystate,bytes);    /* global state array for all the keys */
>   @END
>   #define SEND_HWMSG_INJECTED    0x01
> +#define SEND_HWMSG_ONLY_RAW    0x02
> +#define SEND_HWMSG_SKIP_RAW    0x04
>
> /* Get a message from the current queue */

In lieu of other feedback, I still feel like this is kind of awkward.

Supposedly (though in practice I still don't see why this needs to be 
true) raw input and window-message input should be sending two different 
sets of data. If that's the case, it feels more sensible to me to 
actually split them into two separate calls to __wine_send_input() in 
every case, not just when we actually have native raw input support. 
That is, "no native raw input support" is where we have to work around 
an insufficiency in the lower level, and we should design the code 
around the "correct" case.

I don't know what the actual performance penalty of making two separate 
wineserver calls is, or whether it's worth caring about for systems that 
don't support the Xinput extension.

If nothing else, I'd suggest to change these flags to something more 
like SEND_HWMSG_WINDOW and SEND_HWMSG_RAW, and then use the combination 
of both in the cases where we have only one source of input.

ἔρρωσο,
Zeb
> diff --git a/server/queue.c b/server/queue.c
> index b5e17be18fb..418cd7e524e 100644
> --- a/server/queue.c
> +++ b/server/queue.c
> @@ -1598,7 +1598,7 @@ static int send_hook_ll_message( struct desktop *desktop, struct message *hardwa
>
> /* queue a hardware message for a mouse event */
>   static int queue_mouse_message( struct desktop *desktop, user_handle_t win, const hw_input_t *input,
> -
>                            unsigned int origin, struct msg_queue *sender )
> -
>                            unsigned int origin, struct msg_queue *sender, unsigned int req_flags )
> {
>     const struct rawinput_device *device;
>     struct hardware_msg_data *msg_data;
>
> @@ -1651,7 +1651,8 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons
>           y = desktop->cursor.y;
>       }
> - if ((device = current->process->rawinput_mouse))
> - if ((device = current->process->rawinput_mouse) &&
> -
>    !(req_flags & SEND_HWMSG_SKIP_RAW))
> {
>     if (!(msg = alloc_hardware_message( input->mouse.info, source, time ))) return 0;
>     msg_data = msg->data;
>
> @@ -1670,6 +1671,9 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons
>           queue_hardware_message( desktop, msg, 0 );
>       }
> - if (req_flags & SEND_HWMSG_ONLY_RAW)
> -
>    return 0;
> -
> for (i = 0; i < ARRAY_SIZE( messages ); i++)
> {
>     if (!messages[i]) continue;
>
> @@ -1701,7 +1705,7 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons
>
> /* queue a hardware message for a keyboard event */
>   static int queue_keyboard_message( struct desktop *desktop, user_handle_t win, const hw_input_t *input,
> -
>                               unsigned int origin, struct msg_queue *sender )
> -
>                               unsigned int origin, struct msg_queue *sender, unsigned int req_flags )
> {
>     struct hw_msg_source source = { IMDT_KEYBOARD, origin };
>     const struct rawinput_device *device;
>
> @@ -1777,7 +1781,8 @@ static int queue_keyboard_message( struct desktop *desktop, user_handle_t win, c
>           break;
>       }
> - if ((device = current->process->rawinput_kbd))
> - if ((device = current->process->rawinput_kbd) &&
> -
>    !(req_flags & SEND_HWMSG_SKIP_RAW))
> {
>     if (!(msg = alloc_hardware_message( input->kbd.info, source, time ))) return 0;
>     msg_data = msg->data;
>
> @@ -1795,6 +1800,9 @@ static int queue_keyboard_message( struct desktop *desktop, user_handle_t win, c
>           queue_hardware_message( desktop, msg, 0 );
>       }
> - if (req_flags & SEND_HWMSG_ONLY_RAW)
> -
>    return 0;
> -
> if (!(msg = alloc_hardware_message( input->kbd.info, source, time ))) return 0;
> msg_data = msg->data;
>
> @@ -2351,10 +2359,10 @@ DECL_HANDLER(send_hardware_message)
>       switch (req->input.type)
>       {
>       case INPUT_MOUSE:
> -
>    reply->wait = queue_mouse_message( desktop, req->win, &req->input, origin, sender );
> -
>    reply->wait = queue_mouse_message( desktop, req->win, &req->input, origin, sender, req->flags );
>     break;
> case INPUT_KEYBOARD:
> -
>    reply->wait = queue_keyboard_message( desktop, req->win, &req->input, origin, sender );
> -
>    reply->wait = queue_keyboard_message( desktop, req->win, &req->input, origin, sender, req->flags );
>     break;
> case INPUT_HARDWARE:
>     queue_custom_hardware_message( desktop, req->win, origin, &req->input );