Opened 14 years ago

Closed 12 years ago

Last modified 12 years ago

#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)

ConfineCursorForFullScreen.patch (542 bytes ) - added by Hemu 14 years ago.
proposed patch
CursorConfineWindows.patch (1.3 KB ) - added by Andrew 14 years ago.
AltTab_GrabInput_fix.patch (475 bytes ) - added by Jay Weisskopf 14 years ago.

Download all attachments as: .zip

Change History (36)

comment:1 by Philip Taylor, 14 years ago

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

comment:2 by Hemu, 14 years ago

Owner: set to Hemu
Status: newassigned

by Hemu, 14 years ago

proposed patch

comment:3 by Hemu, 14 years ago

Keywords: review added; simple removed
Resolution: fixed
Status: assignedclosed

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 Philip Taylor, 14 years ago

Resolution: fixed
Status: closedreopened

(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 Hemu, 14 years ago

Status: reopenednew

comment:6 by Hemu, 14 years ago

Status: newassigned

comment:7 by Hemu, 14 years ago

Owner: Hemu removed
Status: assignednew

comment:8 by Philip Taylor, 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 Jay Weisskopf, 14 years ago

Owner: set to Jay Weisskopf
Status: newassigned

comment:10 by Andrew, 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 Jay Weisskopf, 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 Jay Weisskopf, 14 years ago

Keywords: review added

comment:13 by Andrew, 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 Andrew, 14 years ago

Attachment: CursorConfineWindows.patch added

comment:14 by Jay Weisskopf, 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 Andrew, 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.

comment:16 by Philip Taylor, 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.)

comment:17 by Andrew, 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.

in reply to:  16 comment:18 by Jay Weisskopf, 14 years ago

Replying to Philip:

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?

I believe this would also be necessary with real SDL.

comment:19 by Andrew, 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:20 by (none), 14 years ago

Milestone: Unclassified

Milestone Unclassified deleted

comment:21 by Andrew, 14 years ago

Milestone: OS Alpha 2

comment:22 by Kieran P, 14 years ago

Milestone: OS Alpha 2OS Alpha 3

by Jay Weisskopf, 14 years ago

Attachment: AltTab_GrabInput_fix.patch added

comment:23 by Jay Weisskopf, 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 Jay Weisskopf, 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 Kieran P, 14 years ago

Priority: minormajor

in reply to:  17 comment:26 by Philip Taylor, 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 philip, 14 years ago

Resolution: fixed
Status: assignedclosed

(In [8430]) Preserve cursor-grabbing across alt-tabs on Windows (probably fixes #508).

comment:28 by Morvin, 12 years ago

Milestone: Alpha 3Backlog
Priority: Should HaveNice to Have
Resolution: fixed
Status: closedreopened
Type: taskdefect

comment:29 by Morvin, 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 Jonathan Waller, 12 years ago

Keywords: review removed

in reply to:  29 comment:31 by historic_bruno, 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 historic_bruno, 12 years ago

Resolution: fixed
Status: reopenedclosed

comment:33 by leper, 12 years ago

Milestone: BacklogAlpha 3
Note: See TracTickets for help on using tickets.