[PATCH v2 0/1] MR1250: Draft: win32u: Initialize the server's cursor position from the user driver's.
The server's idea of the cursor position is never really initialized. Normally that's not a problem, since the first mouse movement and a number of other X events will update it. But in the case where the user never moves the mouse, it can remain out of sync. That manifests as `GetCursorPos` bouncing between the server's incorrect value and the real one from the user driver, depending on the age of the position on the server. I've marked this as a draft for two reasons: 1. I don't quite know where this belongs. We want to do it early in the lifespan of a user driver but after there's a desktop, so I've put it behind a `pthread_once` called from `get_desktop_window`. 2. I'm calling `NtUserSetCursorPos` to update the server, rather than a direct `set_cursor` request, because it contains some DPI logic. That has the side effect of potentially warping the cursor. That's not desirable or necessary, but it shouldn't be a problem. It's easily factored around if we want to avoid it. Any guidance would be greatly appreciated! -- v2: win32u: Initialize the server's cursor position from the user driver's. https://gitlab.winehq.org/wine/wine/-/merge_requests/1250
From: Tim Clem <tclem(a)codeweavers.com> Until the user moves the mouse, the server's idea of the cursor position is likely incorrect. Very early in the lifespan of a user driver (after desktop creation), sync the server with the result of the driver's GetCursorPos. --- dlls/win32u/input.c | 29 +++++++++++++++++++++++++++++ dlls/win32u/win32u_private.h | 1 + dlls/win32u/winstation.c | 1 + 3 files changed, 31 insertions(+) diff --git a/dlls/win32u/input.c b/dlls/win32u/input.c index 5ba7d151f57..8ab33619dd1 100644 --- a/dlls/win32u/input.c +++ b/dlls/win32u/input.c @@ -30,6 +30,7 @@ #pragma makedep unix #endif +#include <pthread.h> #include "ntstatus.h" #define WIN32_NO_STATUS #include "win32u_private.h" @@ -39,6 +40,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(win); WINE_DECLARE_DEBUG_CHANNEL(keyboard); +WINE_DECLARE_DEBUG_CHANNEL(cursor); static const WCHAR keyboard_layouts_keyW[] = { @@ -2149,3 +2151,30 @@ void toggle_caret( HWND hwnd ) if (ret && !hidden) display_caret( hwnd, &r ); } + +static void sync_cursor_pos(void) +{ + POINT pt = { 0 }; + if (user_driver->pGetCursorPos( &pt )) + { + TRACE_(cursor)( "syncing initial cursor position at (%d, %d)\n", pt.x, pt.y ); + NtUserSetCursorPos( pt.x, pt.y ); + } + else + { + WARN_(cursor)( "initial GetCursorPos failed; cursor may be out of sync with the server\n" ); + } +} + +/*********************************************************************** + * init_server_cursor_pos + * + * Update the server's cursor location with the actual value from the + * user driver. Until the user moves the mouse, the server's location + * is likely incorrect. + */ +void init_server_cursor_pos(void) +{ + static pthread_once_t init_once = PTHREAD_ONCE_INIT; + pthread_once( &init_once, sync_cursor_pos ); +} diff --git a/dlls/win32u/win32u_private.h b/dlls/win32u/win32u_private.h index 4be068b968d..5800804f6cc 100644 --- a/dlls/win32u/win32u_private.h +++ b/dlls/win32u/win32u_private.h @@ -272,6 +272,7 @@ extern HWND get_capture(void) DECLSPEC_HIDDEN; extern BOOL get_cursor_pos( POINT *pt ) DECLSPEC_HIDDEN; extern HWND get_focus(void) DECLSPEC_HIDDEN; extern DWORD get_input_state(void) DECLSPEC_HIDDEN; +extern void init_server_cursor_pos(void) DECLSPEC_HIDDEN; extern BOOL WINAPI release_capture(void) DECLSPEC_HIDDEN; extern BOOL set_capture_window( HWND hwnd, UINT gui_flags, HWND *prev_ret ) DECLSPEC_HIDDEN; extern BOOL set_caret_blink_time( unsigned int time ) DECLSPEC_HIDDEN; diff --git a/dlls/win32u/winstation.c b/dlls/win32u/winstation.c index e606047a79c..33a3b7f65c8 100644 --- a/dlls/win32u/winstation.c +++ b/dlls/win32u/winstation.c @@ -501,6 +501,7 @@ HWND get_desktop_window(void) ERR_(win)( "failed to create desktop window\n" ); register_builtin_classes(); + init_server_cursor_pos(); return UlongToHandle( thread_info->top_window ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1250
NtUserSetCursorPos will ask the driver to change the cursor position too, and so if the user is currently moving the mouse and GetCursorPos retrieve a slightly outdated position when win32u loads, I'm afraid that this could possibly cause some stutter? Then `winex11` guards against such scenario as it only allows cursor to be moved when it is also grabbed, but I don't think it's the case in other drivers. I'm also not completely sure to understand why this is an initialization issue only. We currently only receive mouse input when the mouse moves over a Wine window (or when it's been grabbed but let's ignore that case). So, whenever the user is doing something else, we don't receive any mouse input and `GetCursorPos` can very well diverge. If that causes some issues, for some reason, I'd think that we should maybe instead drop the `GetCursorPos` driver callback altogether and its cache mechanism, which is obviously bogus. This means that applications won't be able to get the true cursor position when it's outside of a Wine application but that would make everything more consistent from the Win32 point of view. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1250#note_13049
I'm afraid that this could possibly cause some stutter?
Yes, I agree that is a potential issue. I considered factoring most of `NtUserSetCursorPos` into a function that does everything but call the driver, and then call that from the sync function.
I'm also not completely sure to understand why this is an initialization issue only.
Absolutely a valid point. I assume the current caching mechanism was for performance reasons, for games that call `GetCursorPos` many times a second. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1250#note_13051
On Thu Nov 3 19:56:13 2022 +0000, Tim Clem wrote:
I'm afraid that this could possibly cause some stutter? Yes, I agree that is a potential issue. I considered factoring most of `NtUserSetCursorPos` into a function that does everything but call the driver, and then call that from the sync function. I'm also not completely sure to understand why this is an initialization issue only. Absolutely a valid point. I assume the current caching mechanism was for performance reasons, for games that call `GetCursorPos` many times a second. I believe I looked and it's been there for ages. Originally `GetCursorPos` was calling the graphics drivers all the time, and updated wineserver position too after that.
The input handling has been refactored and I believe it's been left there to avoid changing too much the behavior but I'm not completely convinced it's still useful. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1250#note_13061
On Thu Nov 3 21:00:10 2022 +0000, Rémi Bernon wrote:
I believe I looked and it's been there for ages. Originally `GetCursorPos` was calling the graphics drivers all the time, and updated wineserver position too after that. The input handling has been refactored and I believe it's been left there to avoid changing too much the behavior but I'm not completely convinced it's still useful. For additional context, this came up when launching a game full-screen, so `GetCursorPos` should be accurate. There are certain conditions where an `EnterNotify` message will be ignored if a window is created under the cursor - namely if the Wine window is a direct child of the root window and has a GL/Vulkan client subwindow. The `EnterNotify` would normally update the server's cache, but since it's ignored (and since we don't get `EnterNotify` events for client windows), it stays stale. I could the problem from that angle instead.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1250#note_13063
Closing this in favor of another approach, or perhaps the medium-term removal of the caching mechanism. Thanks, Rémi! -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1250#note_13957
This merge request was closed by Tim Clem. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1250
participants (3)
-
Rémi Bernon -
Tim Clem
-
Tim Clem (＠tclem)