The idea here is to track the cursor changes on the server side, ultimately [1] using a private message to notify user drivers of changes to apply, instead of tracking the changes in user drivers with throttle mechanisms.
This would greatly simplify winex11 mouse message handling as we would not need to try syncing the window cursor on every mouse move, and ultimately it would also make it possible to have a single thread listening to input in the background. Currently the input thread needs to own the windows that are under the cursor to be able to set it.
Having a single input thread (per-process at least, but ideally and ultimately per-prefix), will solve several issues with input not being received when the foreground thread isn't actively checking their message, as well as issues with DInput incorrectly checking window messages when it should not.
[1] See https://gitlab.winehq.org/rbernon/wine/-/merge_requests/8/commits
From: Rémi Bernon rbernon@codeweavers.com
--- server/queue.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/server/queue.c b/server/queue.c index 07a31b47370..8839b0e47f1 100644 --- a/server/queue.c +++ b/server/queue.c @@ -403,7 +403,7 @@ static struct message *alloc_hardware_message( lparam_t info, struct hw_msg_sour return msg; }
-static int update_desktop_cursor_pos( struct desktop *desktop, int x, int y ) +static int update_desktop_cursor_pos( struct desktop *desktop, user_handle_t win, int x, int y ) { int updated;
@@ -426,7 +426,7 @@ static void set_cursor_pos( struct desktop *desktop, int x, int y )
if ((device = current->process->rawinput_mouse) && (device->flags & RIDEV_NOLEGACY)) { - update_desktop_cursor_pos( desktop, x, y ); + update_desktop_cursor_pos( desktop, 0, x, y ); return; }
@@ -1408,11 +1408,11 @@ static void update_input_key_state( struct desktop *desktop, unsigned char *keys }
/* update the desktop key state according to a mouse message flags */ -static void update_desktop_mouse_state( struct desktop *desktop, unsigned int flags, +static void update_desktop_mouse_state( struct desktop *desktop, unsigned int flags, user_handle_t win, int x, int y, lparam_t wparam ) { if (flags & MOUSEEVENTF_MOVE) - update_desktop_cursor_pos( desktop, x, y ); + update_desktop_cursor_pos( desktop, win, x, y ); if (flags & MOUSEEVENTF_LEFTDOWN) update_input_key_state( desktop, desktop->keystate, WM_LBUTTONDOWN, wparam ); if (flags & MOUSEEVENTF_LEFTUP) @@ -1580,7 +1580,7 @@ static void queue_hardware_message( struct desktop *desktop, struct message *msg if (msg->msg == WM_MOUSEMOVE) { prepend_cursor_history( msg->x, msg->y, msg->time, msg_data->info ); - if (update_desktop_cursor_pos( desktop, msg->x, msg->y )) always_queue = 1; + if (update_desktop_cursor_pos( desktop, msg->win, msg->x, msg->y )) always_queue = 1; } if (desktop->keystate[VK_LBUTTON] & 0x80) msg->wparam |= MK_LBUTTON; if (desktop->keystate[VK_MBUTTON] & 0x80) msg->wparam |= MK_MBUTTON; @@ -1835,7 +1835,7 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons
if ((device = current->process->rawinput_mouse) && (device->flags & RIDEV_NOLEGACY)) { - update_desktop_mouse_state( desktop, flags, x, y, input->mouse.data << 16 ); + update_desktop_mouse_state( desktop, flags, win, x, y, input->mouse.data << 16 ); return 0; }
From: Rémi Bernon rbernon@codeweavers.com
--- server/queue.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/server/queue.c b/server/queue.c index 8839b0e47f1..770ed72ce43 100644 --- a/server/queue.c +++ b/server/queue.c @@ -405,6 +405,8 @@ static struct message *alloc_hardware_message( lparam_t info, struct hw_msg_sour
static int update_desktop_cursor_pos( struct desktop *desktop, user_handle_t win, int x, int y ) { + struct thread_input *input; + struct thread *thread; int updated;
x = max( min( x, desktop->cursor.clip.right - 1 ), desktop->cursor.clip.left ); @@ -414,6 +416,21 @@ static int update_desktop_cursor_pos( struct desktop *desktop, user_handle_t win desktop->cursor.y = y; desktop->cursor.last_change = get_tick_count();
+ if (win && (thread = get_window_thread( win ))) + { + input = thread->queue->input; + release_object( thread ); + } + else input = desktop->foreground_input; + + if (input && input->capture) + win = input->capture; + else if (!win || !is_window_visible( win ) || is_window_transparent( win )) + win = shallow_window_from_point( desktop, x, y ); + + if (win != desktop->cursor.win) updated = 1; + desktop->cursor.win = win; + return updated; }
@@ -1503,7 +1520,7 @@ static user_handle_t find_hardware_message_window( struct desktop *desktop, stru struct message *msg, unsigned int *msg_code, struct thread **thread ) { - user_handle_t win = 0; + user_handle_t win = desktop->cursor.win;
*thread = NULL; *msg_code = msg->msg; @@ -1519,11 +1536,8 @@ static user_handle_t find_hardware_message_window( struct desktop *desktop, stru if (*msg_code < WM_SYSKEYDOWN) *msg_code += WM_SYSKEYDOWN - WM_KEYDOWN; } } - else if (!input || !(win = input->capture)) /* mouse message */ + else if (!input || !input->capture) /* mouse message */ { - if (is_window_visible( msg->win ) && !is_window_transparent( msg->win )) win = msg->win; - else win = shallow_window_from_point( desktop, msg->x, msg->y ); - *thread = window_thread_from_point( win, msg->x, msg->y ); }
@@ -1609,9 +1623,6 @@ static void queue_hardware_message( struct desktop *desktop, struct message *msg } input = thread->queue->input;
- if (win != desktop->cursor.win) always_queue = 1; - desktop->cursor.win = win; - if (!always_queue || merge_message( input, msg )) free_message( msg ); else {
From: Rémi Bernon rbernon@codeweavers.com
--- server/queue.c | 2 +- server/user.h | 1 + server/window.c | 3 +++ 3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/server/queue.c b/server/queue.c index 770ed72ce43..e37afb4dc2d 100644 --- a/server/queue.c +++ b/server/queue.c @@ -403,7 +403,7 @@ static struct message *alloc_hardware_message( lparam_t info, struct hw_msg_sour return msg; }
-static int update_desktop_cursor_pos( struct desktop *desktop, user_handle_t win, int x, int y ) +int update_desktop_cursor_pos( struct desktop *desktop, user_handle_t win, int x, int y ) { struct thread_input *input; struct thread *thread; diff --git a/server/user.h b/server/user.h index 280da454d07..24533b64605 100644 --- a/server/user.h +++ b/server/user.h @@ -192,6 +192,7 @@ extern void set_process_default_desktop( struct process *process, struct desktop extern void close_process_desktop( struct process *process ); extern void set_thread_default_desktop( struct thread *thread, struct desktop *desktop, obj_handle_t handle ); extern void release_thread_desktop( struct thread *thread, int close ); +extern int update_desktop_cursor_pos( struct desktop *desktop, user_handle_t win, int x, int y );
/* checks if two rectangles are identical */ static inline int is_rect_equal( const rectangle_t *rect1, const rectangle_t *rect2 ) diff --git a/server/window.c b/server/window.c index f713ed224aa..6489b2bd764 100644 --- a/server/window.c +++ b/server/window.c @@ -2495,6 +2495,9 @@ DECL_HANDLER(set_window_pos) reply->surface_win = top->handle; reply->needs_update = !!(top->paint_flags & (PAINT_HAS_PIXEL_FORMAT | PAINT_PIXEL_FORMAT_CHILD)); } + + /* update the desktop cursor window from the cursor position */ + update_desktop_cursor_pos( win->desktop, 0, win->desktop->cursor.x, win->desktop->cursor.y ); }
From: Rémi Bernon rbernon@codeweavers.com
--- server/queue.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/server/queue.c b/server/queue.c index e37afb4dc2d..dd52fc43716 100644 --- a/server/queue.c +++ b/server/queue.c @@ -3277,14 +3277,16 @@ DECL_HANDLER(set_cursor) { struct msg_queue *queue = get_current_queue(); struct thread_input *input; + struct desktop *desktop;
if (!queue) return; input = queue->input; + desktop = input->desktop;
reply->prev_handle = input->cursor; reply->prev_count = input->cursor_count; - reply->prev_x = input->desktop->cursor.x; - reply->prev_y = input->desktop->cursor.y; + reply->prev_x = desktop->cursor.x; + reply->prev_y = desktop->cursor.y;
if (req->flags & SET_CURSOR_HANDLE) { @@ -3302,12 +3304,10 @@ DECL_HANDLER(set_cursor) } if (req->flags & SET_CURSOR_POS) { - set_cursor_pos( input->desktop, req->x, req->y ); + set_cursor_pos( desktop, req->x, req->y ); } if (req->flags & (SET_CURSOR_CLIP | SET_CURSOR_NOCLIP)) { - struct desktop *desktop = input->desktop; - /* only the desktop owner can set the message */ if (req->clip_msg && get_top_window_owner(desktop) == current->process) desktop->cursor.clip_msg = req->clip_msg; @@ -3315,10 +3315,10 @@ DECL_HANDLER(set_cursor) set_clip_rectangle( desktop, (req->flags & SET_CURSOR_NOCLIP) ? NULL : &req->clip, 0 ); }
- reply->new_x = input->desktop->cursor.x; - reply->new_y = input->desktop->cursor.y; - reply->new_clip = input->desktop->cursor.clip; - reply->last_change = input->desktop->cursor.last_change; + reply->new_x = desktop->cursor.x; + reply->new_y = desktop->cursor.y; + reply->new_clip = desktop->cursor.clip; + reply->last_change = desktop->cursor.last_change; }
/* Get the history of the 64 last cursor positions */
From: Rémi Bernon rbernon@codeweavers.com
--- server/queue.c | 11 +++++++++++ server/user.h | 1 + 2 files changed, 12 insertions(+)
diff --git a/server/queue.c b/server/queue.c index dd52fc43716..211fc295e41 100644 --- a/server/queue.c +++ b/server/queue.c @@ -434,6 +434,11 @@ int update_desktop_cursor_pos( struct desktop *desktop, user_handle_t win, int x return updated; }
+static void update_desktop_cursor_handle( struct desktop *desktop, user_handle_t handle ) +{ + desktop->cursor.handle = handle; +} + /* set the cursor position and queue the corresponding mouse message */ static void set_cursor_pos( struct desktop *desktop, int x, int y ) { @@ -3315,6 +3320,12 @@ DECL_HANDLER(set_cursor) set_clip_rectangle( desktop, (req->flags & SET_CURSOR_NOCLIP) ? NULL : &req->clip, 0 ); }
+ if (req->flags & (SET_CURSOR_HANDLE | SET_CURSOR_COUNT)) + { + if (input->cursor_count < 0) update_desktop_cursor_handle( desktop, 0 ); + else update_desktop_cursor_handle( desktop, input->cursor ); + } + reply->new_x = desktop->cursor.x; reply->new_y = desktop->cursor.y; reply->new_clip = desktop->cursor.clip; diff --git a/server/user.h b/server/user.h index 24533b64605..4d86d201db5 100644 --- a/server/user.h +++ b/server/user.h @@ -60,6 +60,7 @@ struct global_cursor unsigned int clip_msg; /* message to post for cursor clip changes */ unsigned int last_change; /* time of last position change */ user_handle_t win; /* window that contains the cursor */ + user_handle_t handle; /* last set cursor handle */ };
struct desktop
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=128799
Your paranoid android.
=== debian11 (32 bit report) ===
user32: win.c:10266: Test failed: button under static window didn't get WM_LBUTTONUP
On Thu Jan 26 12:45:33 2023 +0000, **** wrote:
Marvin replied on the mailing list:
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=128799 Your paranoid android. === debian11 (32 bit report) === user32: win.c:10266: Test failed: button under static window didn't get WM_LBUTTONUP
Seems like a genuine bug, also on Gitlab, I'll have a look.
I'm having a bit of trouble with one user32:msg test, which is unhappy about the internal message being posted at an unexpected time, changing the queue status.
I'm not completely sure how to better post asynchronous work to user drivers, it seems a bit brittle that internal messages are somehow visible to the client, but then the alternative mechanisms I can think about also seem a bit too complicated and not very appealing (send a X11 event, use an APC?).
This merge request was closed by Rémi Bernon.
-
Marvin -
Rémi Bernon -
Rémi Bernon (＠rbernon)