#508 closed defect (fixed)
Cursor should be restricted to the game window when fullscreen on Win32
Reported by: | Philip Taylor | Owned by: | Jay Weisskopf |
---|---|---|---|
Priority: | Nice to Have | Milestone: | Alpha 3 |
Component: | Core engine | Keywords: | |
Cc: | Patch: |
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 (3)
Change History (36)
comment:1 by , 14 years ago
comment:2 by , 14 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:3 by , 14 years ago
Keywords: | review added; simple removed |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
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 by , 14 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
(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:5 by , 14 years ago
Status: | reopened → new |
---|
comment:6 by , 14 years ago
Status: | new → assigned |
---|
comment:7 by , 14 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:8 by , 14 years ago
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:9 by , 14 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:10 by , 14 years ago
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 by , 14 years ago
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:12 by , 14 years ago
Keywords: | review added |
---|
comment:13 by , 14 years ago
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.
by , 14 years ago
Attachment: | CursorConfineWindows.patch added |
---|
comment:14 by , 14 years ago
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 by , 14 years ago
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.
follow-up: 18 comment:16 by , 14 years ago
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.)
follow-up: 26 comment:17 by , 14 years ago
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 by , 14 years ago
comment:19 by , 14 years ago
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:21 by , 14 years ago
Milestone: | → OS Alpha 2 |
---|
comment:22 by , 14 years ago
Milestone: | OS Alpha 2 → OS Alpha 3 |
---|
by , 14 years ago
Attachment: | AltTab_GrabInput_fix.patch added |
---|
comment:23 by , 14 years ago
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 by , 14 years ago
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:25 by , 14 years ago
Priority: | minor → major |
---|
comment:26 by , 14 years ago
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 by , 14 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:28 by , 12 years ago
Milestone: | Alpha 3 → Backlog |
---|---|
Priority: | Should Have → Nice to Have |
Resolution: | fixed |
Status: | closed → reopened |
Type: | task → defect |
follow-up: 31 comment:29 by , 12 years ago
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:30 by , 12 years ago
Keywords: | review removed |
---|
comment:31 by , 12 years ago
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 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
comment:33 by , 12 years ago
Milestone: | Backlog → Alpha 3 |
---|
(See
lib/sysdep/os/win/wsdl.cpp
for the code that does all the window management.)