[PATCH 0/1] MR825: winex11.drv: optimize repeated ClipCursor calls
At least Doom 64 calls ClipCursor() with identical parameters repeatedly, which seems to cause a considerable overhead. Together with high polling rate mouse input this causes the game to almost freeze while the mouse is being moved. So if the clipping did not change we can bail out early because it will not cause any observable difference anyway. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46976 Signed-off-by: Jan Kl��tzke <jan(a)kloetzke.net> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/825
From: Jan Klötzke <jan(a)kloetzke.net> At least Doom 64 calls ClipCursor() with identical parameters repeatedly, which seems to cause a considerable overhead. Together with high polling rate mouse input this causes the game to almost freeze while the mouse is being moved. So if the clipping did not change we can bail out early because it will not cause any observable difference anyway. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46976 Signed-off-by: Jan Klötzke <jan(a)kloetzke.net> --- dlls/winex11.drv/mouse.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dlls/winex11.drv/mouse.c b/dlls/winex11.drv/mouse.c index 96be81df6e3..6e3b68af454 100644 --- a/dlls/winex11.drv/mouse.c +++ b/dlls/winex11.drv/mouse.c @@ -383,6 +383,10 @@ static BOOL grab_clipping_window( const RECT *clip ) return TRUE; /* don't clip in the desktop process */ if (!data) return FALSE; + + if (data->clip_hwnd && EqualRect( clip, &clip_rect )) + return TRUE; + if (!(clip_window = init_clip_window())) return TRUE; RtlInitUnicodeString( &class_name, messageW ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/825
I don't think this will work very well, there's several things that can happen if you do this: * X11 may override cursor grab without notifying the application, you will end up with the mouse free to move when the game and Wine still believe it's constrained. * Several games are doing `ClipCursor(NULL); ClipCursor(&rect);` in sequence, effectively changing the rect on every frame. I believe the slowdown comes from the Win32 window creation, not really from `XGrabPointer` requests, so that should be optimized instead. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/825#note_8202
Indeed, the overhead does seem to come from the Win32 window creation. Interestingly the allocation of a new window every time the clipping changes, was introduced in dc801b9831f990c5b9e61da93c258404820b2b38 to "avoid stale notifications". Now I stared at all the code for almost two hours and don't yet get what notification could be outdated. Maybe this is not relevant any more? I will gladly refactor the code to allocate the Win32 window only once but I guess some hints could be helpful to not fall into traps that have been fixed some 11 years ago... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/825#note_8299
Indeed, the overhead does seem to come from the Win32 window creation. Interestingly the allocation of a new window every time the clipping changes, was introduced in dc801b9831f990c5b9e61da93c258404820b2b38 to "avoid stale notifications". Now I stared at all the code for almost two hours and don't yet get what notification could be outdated. Maybe this is not relevant any more?
It's always hard to tell, especially with `winex11`.
I will gladly refactor the code to allocate the Win32 window only once but I guess some hints could be helpful to not fall into traps that have been fixed some 11 years ago...
Well, feel free to try but it probably won't be easy. I tried for Proton to move the clipping logic out of the main process, to avoid this overhead, but it caused subtle issues with some games and we reverted the changes. I believe `winex11` mouse handling should be globally refactored to avoid blocking applications threads, and that could be part of it, but such a big change will be hard to get accepted. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/825#note_8304
participants (2)
-
Jan Klötzke -
Rémi Bernon