Ticket #508 (closed defect: fixed)

Opened 3 years ago

Last modified 14 months ago

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

ConfineCursorForFullScreen.patch (542 bytes) - added by moo 3 years ago.
proposed patch
CursorConfineWindows.patch (1.3 KB) - added by wacko 3 years ago.
AltTab_GrabInput_fix.patch (475 bytes) - added by Jayschwa 3 years ago.

Change History

comment:1 Changed 3 years ago by Philip

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

comment:2 Changed 3 years ago by moo

  • Owner set to moo
  • Status changed from new to assigned

Changed 3 years ago by moo

proposed patch

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:5 Changed 3 years ago by moo

  • Status changed from reopened to new

comment:6 Changed 3 years ago by moo

  • Status changed from new to assigned

comment:7 Changed 3 years ago by moo

  • Owner moo deleted
  • Status changed from assigned to new

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:9 Changed 3 years ago by Jayschwa

  • Owner set to Jayschwa
  • Status changed from new to assigned

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:12 Changed 3 years ago by Jayschwa

  • Keywords review added

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.

Changed 3 years ago by wacko

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

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 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:21 Changed 3 years ago by wacko

  • Milestone set to OS Alpha 2

comment:22 Changed 3 years ago by k776

  • Milestone changed from OS Alpha 2 to OS Alpha 3

Changed 3 years ago by Jayschwa

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:25 Changed 3 years ago by k776

  • Priority changed from minor to major

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

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

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:30 Changed 15 months ago by quantumstate

  • Keywords review removed

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

comment:33 Changed 14 months ago by leper

  • Milestone changed from Backlog to Alpha 3
Note: See TracTickets for help on using tickets.