Lauri Kenttä wrote:
When the cursor position is clipped with ClipCursor, the cursor should be confined to that area. This patch fixes cursor position clipping in X11DRV_ClipCursor, X11DRV_GetCursorPos, X11DRV_SetCursorPos and X11DRV_send_mouse_input and makes the mouse warp to the new position if moved outside the clip rect.
The previous try fixed only clipping (not warping) and only in GetCursorPos, but the problem wasn't that small after all.
dlls/winex11.drv/mouse.c | 52 ++++++++++++++++++++++++++++++++++++--------- 1 files changed, 41 insertions(+), 11 deletions(-)
You changing way too many things without a single test. Lots of those areas you touch will break oh so many apps.
For starters write some tests and verify what you doing is correct. You should be able to write tests for all injected messages (send_mouse_event). If you can't make an automated test, write a visual test for the rest and send to wine-devel. Especially pay attention to all messages sent, and order of calling hook, sending messages, modifying values returned from GetCursorPos(). You breaking that.
And don't try to keep pointer inside Wine's window - this won't get past AJ.
Vitaliy.
Thanks for your comments, Vitaliy.
At first, I must point out that you're commenting on a patch that has already been superseded. Not that there's much difference, but the new series of patches has some related changes (including a new test) applied before this one.
Vitaliy Margolen wrote:
Lots of those areas you touch will break oh so many apps.
Could you please elaborate that "so many"?
I'm aware that my version causes hooks to be called with clipped coordinates on mouse move, when they should in that case be called with the unclipped ones. Still, please note that the current version calls all hooks (not just on mouse move) with the unclipped coordinates, returns unclipped coordinates from GetCursorPos and even sends incorrect WM_* messages. This is clearly wrong and causes many problems (example: border scrolling is next to impossible in many full-screen games when played on Wine virtual desktop).
I sincerely think my version is less likely to break anything. Surely there are more applications using WM_* and GetCursorPos than hooks.
Maybe I will have time to fix the remaining bugs some day. I will also try to write some Wine tests so that these problems (be they in my version or the old one) are not forgotten.
Vitaliy Margolen wrote:
And don't try to keep pointer inside Wine's window
Anyone could do exactly the same thing "manually" by calling SetCursorPos, so why shouldn't ClipCursor do it automatically? Also, apparently Wine won't (or can't) warp the mouse if it lies outside the virtual desktop.
As I see it, clipping should first be implemented correctly, because that's what Windows applications expect. Then, if this causes problems, we could use a registry key similar to DXGrab to disable (or restrict) XWarpPointer usage.
Lauri Kenttä wrote:
The new series of patches has some related changes (including a new test) applied before this one.
You have some issues in your test as well. The SetCursorPos() calls hooks on Wine because Wine has big issue - it doesn't filter pointer move messages that were caused by Wine itself. What you did in your patch is a half fix the wrong way.
Could you please elaborate that "so many"?
I can't give you all the programs/games that depend on particular behavior - there are big number of them.
I'm aware that my version causes hooks to be called with clipped coordinates on mouse move, when they should in that case be called with the unclipped ones.
This particular change will break all games using dinput when mouse pointer goes outside of virtual desktop.
returns unclipped coordinates from GetCursorPos
This is what native will do inside hook handler. Add some tests.
and even sends incorrect WM_* messages.
Tests please.
This is clearly wrong and causes many problems (example: border scrolling is next to impossible in many full-screen games when played on Wine virtual desktop).
What exactly is wrong with them? And what part are you trying to fix?
I sincerely think my version is less likely to break anything. Surely there are more applications using WM_* and GetCursorPos than hooks.
This is not a reason to break anything. You either fix things without breaking anything else. Or you don't touch the code at all.
Anyone could do exactly the same thing "manually" by calling SetCursorPos, so why shouldn't ClipCursor do it automatically? Also, apparently Wine won't (or can't) warp the mouse if it lies outside the virtual desktop.
Number of reasons: - Who said that Wine should keep the pointer inside virtual desktop? It's there for the exact reason so users can have other apps running outside of Wine at the same time as full-screen game inside Wine. - You doing it wrong. Warping pointer is not the right way. If you really must do it, then you should be using pointer grab. Also you are not dealing with all the extra pointer move messages generated as a result of XWarpPointer().
Vitaliy.
-
Lauri Kenttä -
Vitaliy Margolen