Ticket #508 (closed defect: fixed)
Cursor should be restricted to the game window when fullscreen on Win32
| Reported by: | Philip | Owned by: | Jayschwa |
|---|---|---|---|
| Priority: | Nice to Have | Milestone: | Alpha 3 |
| Component: | Core engine | Keywords: | |
| Cc: |
Description
When running fullscreen on a multi-monitor system, the mouse can easily leave the game window and move onto the other screen, which breaks edge-of-screen scrolling, and if you accidentally click outside then the game minimises which is quite irritating.
When fullscreen, the mouse ought to be restricted to the window (presumably via ClipCursor).
Attachments
Change History
comment:3 Changed 3 years ago by moo
- Keywords review added; simple removed
- Status changed from assigned to closed
- Resolution set to fixed
Simple three line fix in wsdl.cpp in function SDL_SetVideoMode(). Cursor not visible on far right because it is slightly off screen, but it IS still in the game window. Let me know if the intent was for the cursor to remain entirely visible.
comment:4 Changed 3 years ago by Philip
- Status changed from closed to reopened
- Resolution fixed deleted
(Tickets should preferably only be marked as fixed once the fix is in SVN, otherwise the patch might get lost before it's applied.)
Thanks, this seems to do the right thing when it works (there's no need to adjust for the cursor size), but there's a few situations where it doesn't work:
- When starting in fullscreen mode (set windowed=false in %appdata%\0ad\config\local.cfg), it doesn't clip the cursor.
- After alt-tabbing away while in fullscreen, then alt-tabbing back, it loses the clipping.
- When switching from fullscreen to windowed (with alt-enter), it continues to clip the cursor (it shouldn't while windowed).
I can't think of any other cases to test - I don't know if I'm missing some but it'd be good to at least handle these cases robustly.
comment:8 Changed 3 years ago by Philip
- Keywords review removed
To anyone who's wondering at the status: moo said (in email) he no longer has access to an external monitor for testing, so he can't work on this more.
Anyone with the necessary equipment for testing should feel free to try to extend the patch to fix the problems I found (and ideally any I missed).
comment:10 Changed 3 years ago by wacko
I have attached a patch that will resolve the issues Philip talked about and also deal with in you switch from windowed to fullscreen.
comment:11 Changed 3 years ago by Jayschwa
The patch works as advertised, but I dislike that the change occurs in SDL_SetVideoMode(). In my opinion, the emulated SDL functions should behave exactly the same as real SDL; as far as I know, SDL_SetVideoMode() does not grab the cursor in real SDL.
I had a patch applied a couple weeks ago that calls SDL_WM_GrabInput() after SDL_SetVideoMode() is called. The only outstanding issue is that the cursor stops working after an Alt+Tab in fullscreen. To resolve this, I think an additional SDL_WM_GrabInput() should be added whenever the game regains focus. SDL_ActiveEvent exists for detecting this and I believe the game already pays attention to them, so it should just be a matter of popping the GrabInput? call in there.
comment:13 Changed 3 years ago by wacko
I disagree with the attached patch as its not required. SDL_WM_GrabInput() is buggy and does not work correctly in OS X if you are going to use it.
I also fixed SDL_WM_GrabInput() as it was doing a bunch of stuff it did not need to do on windows.
comment:14 Changed 3 years ago by Jayschwa
GetWindowRect() appears to a good simplification for some of the lines in SDL_WM_GrabInput(). However, your patch will break it in a couple ways:
- Nothing will happen if the function is called in a windowed mode (ConfineCursor() immediately returns when fullscreen is false).
- Nothing will happen if the function is called with SDL_GRAB_OFF.
I also still think it's not good to have ClipCursor() inside of SDL_SetVideoMode() since that deviates from real SDL's behavior.
comment:15 Changed 3 years ago by wacko
there is no reason anything need to happen when in windowed mode. As well as there is no reason why you need to call anything for SDL_GRAB_OFF.
Also I am not calling it in SetVideoMode? I am calling it in onActivate.
comment:16 follow-up: ↓ 18 Changed 3 years ago by Philip
Hmm, I'm a little confused now as to what the real problems are and what correctly solves them...
Replying to Jayschwa:
I think an additional SDL_WM_GrabInput() should be added whenever the game regains focus.
Is this needed for real SDL on any platform, or is it just a workaround for wsdl bugs?
Replying to wacko:
SDL_WM_GrabInput() is buggy and does not work correctly in OS X if you are going to use it.
What is the problem on OS X?
It seems good to mirror the real SDL behaviour on Windows where possible, to minimise the pain if we ever switch back to real SDL. Changing from GetClientRect to GetWindowRect would give different behaviour in windowed mode (since the latter includes the title bar and window border etc), and real SDL seems to use GetClientRect, so I think it's best to stick with that. (We might want to start restricting the cursor in windowed mode (at least as an option for the user), so we shouldn't knowingly regress that functionality now when it's easy enough not to.)
comment:17 follow-up: ↓ 26 Changed 3 years ago by wacko
Saying now that we may want to confine the cursor in Windowed mode... seems be out of the scope of this particular issue. I do not know why you would ever want such a feature in relation to this game. So I do not find issue using GetWindowRect? in this case, because I do not think we will ever want to confine the cursor in window mode.
If your saying we should use GetClientRect? because you want to be consistent with SDL then that is fine, but there is nothing wrong with the patch I attached and the scope of this issue.
As for OS X it has been known that SDL_WM_GrabInput has not always well Grabbed in OSX and there has been many cases where people have complained about the inconsistency in its behavior.
comment:18 in reply to: ↑ 16 Changed 3 years ago by Jayschwa
comment:19 Changed 3 years ago by wacko
So in the REAL SDL for windows, SDL can deal with processing of the Windows Events and it calls a function in the WM_ACTIVATE case statement of the processing of these events which calls ClipCursor? in this case WIN_INPUTGRAB which calls code such as the one in my patch.
So assuming that all the other OSs that are actually using the SDL are processing the OS events they too would already be doing something similar. My patch is simulating exactly what the real SDL does by calling a function within the processing of the WM_ACTIVATE event. There is no reason to call this function on any other platform because it should already be doing the right thing.
comment:20 Changed 3 years ago by anonymous
- Milestone Unclassified deleted
Milestone Unclassified deleted
comment:23 Changed 3 years ago by Jayschwa
After testing this function in another project which uses real SDL, I discovered I was wrong. The grab state persists across alt-tabbing and it should not be necessary to call SDL_WM_GrabInput() again.
Taking a cue from wacko's use of OnActivate(), I redid my patch (attached) so that it simply re-asserts the grab state when the game is reactivated.
comment:24 Changed 3 years ago by Jayschwa
I should clarify that the call to SDL_WM_GrabInput() in my patch is inside the emulation code. From the external codebase's perspective, the function is still only called once (in VideoMode.cpp).
comment:26 in reply to: ↑ 17 Changed 3 years ago by Philip
Replying to wacko:
Saying now that we may want to confine the cursor in Windowed mode... seems be out of the scope of this particular issue.
I didn't mean that we should do that - just that if we did, then the change to GetWindowRect in your patch would cause a bug, and there isn't any apparent benefit in that change in any case, so it's better to not change it at all.
Replying to Jayschwa:
Taking a cue from wacko's use of OnActivate(), I redid my patch (attached) so that it simply re-asserts the grab state when the game is reactivated.
Okay.
comment:27 Changed 3 years ago by philip
- Status changed from assigned to closed
- Resolution set to fixed
comment:28 Changed 15 months ago by Morvin
- Priority changed from Should Have to Nice to Have
- Status changed from closed to reopened
- Type changed from task to defect
- Resolution fixed deleted
- Milestone changed from Alpha 3 to Backlog
comment:29 follow-up: ↓ 31 Changed 15 months ago by Morvin
There isn't automated focus on a game's window after game start. User need to click in the window for activating it otherwise if he had another application running in fullscreen mode he won't see the window game. Cursor should be restricted
OS: Windows 7 Ultimate SP1 x64; CPU: i5-2400; Video: nVidia GeForce? 210
comment:31 in reply to: ↑ 29 Changed 14 months ago by historic_bruno
Replying to Morvin:
There isn't automated focus on a game's window after game start. User need to click in the window for activating it otherwise if he had another application running in fullscreen mode he won't see the window game. Cursor should be restricted
OS: Windows 7 Ultimate SP1 x64; CPU: i5-2400; Video: nVidia GeForce? 210
This doesn't sound related to the original ticket, it's probably best to create a new ticket.
Are you using a dual monitor setup by chance?
comment:32 Changed 14 months ago by historic_bruno
- Status changed from reopened to closed
- Resolution set to fixed

(See lib/sysdep/os/win/wsdl.cpp for the code that does all the window management.)