sezero / quakespasm

QuakeSpasm -- A modern, cross-platform Quake game engine based on FitzQuake.
https://sourceforge.net/projects/quakespasm/
GNU General Public License v2.0
227 stars 96 forks source link

Mac Windowed Mode Out Of Focus Input #66

Closed timbergeron closed 1 year ago

timbergeron commented 1 year ago

In latest QS on Mac if you are in windowed mode and you move your mouse over the out of focus QS window, the game receives the input despite the application being out of focus.

sezero commented 1 year ago

I believe this is a duplicate of https://github.com/sezero/quakespasm/issues/48 and already is fixed in development tree as of https://github.com/sezero/quakespasm/commit/88d9a8f45dd8c2685963b3fb54de8605c7f8d6ac

If not, please re-open.

sezero commented 1 year ago

I believe this is a duplicate of #48 and already is fixed in development tree as of 88d9a8f

I may have closed this too hastily, i.e. my statement above may be incorrect.

In any case, please try and rebuild from git (the SDL2 version) and see if the issue persists. If it does, please try replacing the included SDL2 version with the latest official SDL2 release (which is, as of this writing, 2.28.1: https://github.com/libsdl-org/SDL/releases/tag/release-2.28.1)

timbergeron commented 1 year ago

Tried both just now and the issue still persists.

sezero commented 1 year ago

@ericwa: Any insight, please?

timbergeron commented 1 year ago

Upon some further investigation, this only happens on my MacBook Pro M1 (Ventura), not my old Intel MacBook Pro (Monterey)

timbergeron commented 1 year ago

https://github.com/sezero/quakespasm/assets/68674590/de846254-478e-4197-8731-e0692ce8af49

sample video

ericwa commented 1 year ago

I don't have a mac anymore, unfortunately.

On Windows, on a88b597a3d7f5e79e96609cb1f4c39dec7a1fbbe , this does not happen - when the window is alt-tabbed away from and inactive, no mouse input is picked up by QS (even if I mouse over the window, like in the sample video above). I'm assuming this is the desired behaviour.

We don't do anything special to achieve this; we're just not receiving SDL_MOUSEMOTION events from SDL when the window is inactive. We also don't call IN_Deactivate when alt-tabbing away either, so that's not relevant. We also keep SDL relative mouse mode enabled.

Here's an untested workaround:

diff --git a/Quake/in_sdl.c b/Quake/in_sdl.c
index caa7e4dd..429441cc 100644
--- a/Quake/in_sdl.c
+++ b/Quake/in_sdl.c
@@ -1096,6 +1096,17 @@ void IN_SendKeyEvents (void)
 #endif

                case SDL_MOUSEMOTION:
+#if defined(USE_SDL2)
+                       {
+                               SDL_Window* win = SDL_GetMouseFocus();
+                               if (win && !(SDL_GetWindowFlags(win) & SDL_WINDOW_INPUT_FOCUS))
+                               {
+                                       // we received a mouse event without input focus (can happen when alt-tabbed
+                                       // away on macOS, but with the mouse over the window) - ignore it
+                                       break;
+                               }
+                       }
+#endif
                        IN_MouseMotion(event.motion.xrel, event.motion.yrel);
                        break;

This could be moved into a VID helper function similar to VID_HasMouseOrInputFocus but I just kept it in in_sdl.c for simplicity.

If this fixes the issue, ideally SDL would document what is expected, and/or fix this, if it's considered an SDL bug, because they don't currently say in the SDL2 function docs what is expected to happen when the window is alt-tabbed away from: https://github.com/libsdl-org/SDL/blob/SDL2/include/SDL_mouse.h#L201

sezero commented 1 year ago

@timbergeron: Can you test @ericwa's patch?

sezero commented 1 year ago

CC: @slouken, @icculus

timbergeron commented 1 year ago

I added this patch and still the same result :(

icculus commented 1 year ago

Could it be this that caused it? https://github.com/libsdl-org/SDL/commit/2afb49ba9a2edc361a3b35d8e30be977a5bcc0c4

sezero commented 1 year ago

Could it be this that caused it? libsdl-org/SDL@2afb49b

Assuming that @timbergeron originally tested the official qs releases and builds from qs git without modification (@timbergeron: am I correct?), the SDL2 framework included there is based on 2.0.22 (https://github.com/sezero/SDL/tree/2.0.22-backports) which certainly does not have that patch. Only when he replaced the SDL2 framework with latest releases from libsdl, would that patch show its effect, if any.

timbergeron commented 1 year ago

This also appears to be an issue with the linux build as well. Just tested in Ubuntu 64bit. I believe I tested correctly as stated above.

sezero commented 1 year ago

This also appears to be an issue with the linux build as well. Just tested in Ubuntu 64bit. I believe I tested correctly as stated above.

OK, reproduced on i686-linux + SDL2-2.0.22 with a loaded game running and doing an Alt-Tab. This looks like not an SDL issue but an issue in qs itself.

sezero commented 1 year ago

Is the following patch good? @ericwa, @andrei-drexler, @temx: Please review. (Also attaching for convenience: patch-66.txt)

(Edit: DON'T USE. Updated patch is in the next post)

```diff diff --git a/Quake/in_sdl.c b/Quake/in_sdl.c index caa7e4d..0f02a38 100644 --- a/Quake/in_sdl.c +++ b/Quake/in_sdl.c @@ -98,6 +98,7 @@ static int SDLCALL IN_FilterMouseEvents (const SDL_Event *event) return 1; } +static SDL_EventFilter currentFilter = NULL; #if defined(USE_SDL2) static int SDLCALL IN_SDL2_FilterMouseEvents (void *userdata, SDL_Event *event) { @@ -108,28 +109,48 @@ static int SDLCALL IN_SDL2_FilterMouseEvents (void *userdata, SDL_Event *event) static void IN_BeginIgnoringMouseEvents(void) { #if defined(USE_SDL2) - SDL_EventFilter currentFilter = NULL; void *currentUserdata = NULL; SDL_GetEventFilter(¤tFilter, ¤tUserdata); if (currentFilter != IN_SDL2_FilterMouseEvents) + { SDL_SetEventFilter(IN_SDL2_FilterMouseEvents, NULL); + currentFilter = IN_SDL2_FilterMouseEvents; + } #else - if (SDL_GetEventFilter() != IN_FilterMouseEvents) + currentFilter = SDL_GetEventFilter(); + if (currentFilter != IN_FilterMouseEvents) + { SDL_SetEventFilter(IN_FilterMouseEvents); + currentFilter = IN_FilterMouseEvents; + } #endif } static void IN_EndIgnoringMouseEvents(void) { #if defined(USE_SDL2) - SDL_EventFilter currentFilter; void *currentUserdata; if (SDL_GetEventFilter(¤tFilter, ¤tUserdata) == SDL_TRUE) + { + currentFilter = NULL; SDL_SetEventFilter(NULL, NULL); + } #else if (SDL_GetEventFilter() != NULL) + { + currentFilter = NULL; SDL_SetEventFilter(NULL); + } +#endif +} + +static void IN_RestoreMouseEvents(void) +{ +#if defined(USE_SDL2) + SDL_SetEventFilter(currentFilter, NULL); +#else + SDL_SetEventFilter(currentFilter); #endif } @@ -1014,17 +1035,30 @@ void IN_SendKeyEvents (void) #if defined(USE_SDL2) case SDL_WINDOWEVENT: if (event.window.event == SDL_WINDOWEVENT_FOCUS_GAINED) + { + IN_RestoreMouseEvents(); S_UnblockSound(); + } else if (event.window.event == SDL_WINDOWEVENT_FOCUS_LOST) + { + IN_BeginIgnoringMouseEvents(); S_BlockSound(); + } break; #else case SDL_ACTIVEEVENT: if (event.active.state & (SDL_APPINPUTFOCUS|SDL_APPACTIVE)) { if (event.active.gain) + { + IN_RestoreMouseEvents(); S_UnblockSound(); - else S_BlockSound(); + } + else + { + IN_BeginIgnoringMouseEvents(); + S_BlockSound(); + } } break; #endif ```
sezero commented 1 year ago

Cleaner (and finally corrected) patch inlined below (also attached: patch-66-2.txt.) @timbergeron: Please test. @ericwa, @andrei-drexler, @temx: Please review.

diff --git a/Quake/in_sdl.c b/Quake/in_sdl.c
index caa7e4d..48fb4d1 100644
--- a/Quake/in_sdl.c
+++ b/Quake/in_sdl.c
@@ -133,6 +133,30 @@ static void IN_EndIgnoringMouseEvents(void)
 #endif
 }

+static SDL_EventFilter prevFilter = NULL;
+
+static void IN_BeginIgnoringMouseEvents2(void)
+{
+#if defined(USE_SDL2)
+   void *userdata = NULL;
+   SDL_GetEventFilter(&prevFilter, &userdata);
+   SDL_SetEventFilter(IN_SDL2_FilterMouseEvents, NULL);
+   (void) userdata;
+#else
+   prevFilter = SDL_GetEventFilter();
+   SDL_SetEventFilter(IN_FilterMouseEvents);
+#endif
+}
+
+static void IN_RestoreMouseEvents(void)
+{
+#if defined(USE_SDL2)
+   SDL_SetEventFilter(prevFilter, NULL);
+#else
+   SDL_SetEventFilter(prevFilter);
+#endif
+}
+
 #ifdef MACOS_X_ACCELERATION_HACK
 static cvar_t in_disablemacosxmouseaccel = {"in_disablemacosxmouseaccel", "1", CVAR_ARCHIVE};
 static double originalMouseSpeed = -1.0;
@@ -1014,17 +1038,30 @@ void IN_SendKeyEvents (void)
 #if defined(USE_SDL2)
        case SDL_WINDOWEVENT:
            if (event.window.event == SDL_WINDOWEVENT_FOCUS_GAINED)
+           {
+               IN_RestoreMouseEvents();
                S_UnblockSound();
+           }
            else if (event.window.event == SDL_WINDOWEVENT_FOCUS_LOST)
+           {
+               IN_BeginIgnoringMouseEvents2();
                S_BlockSound();
+           }
            break;
 #else
        case SDL_ACTIVEEVENT:
            if (event.active.state & (SDL_APPINPUTFOCUS|SDL_APPACTIVE))
            {
                if (event.active.gain)
+               {
+                   IN_RestoreMouseEvents();
                    S_UnblockSound();
-               else    S_BlockSound();
+               }
+               else
+               {
+                   IN_BeginIgnoringMouseEvents2();
+                   S_BlockSound();
+               }
            }
            break;
 #endif
timbergeron commented 1 year ago

Initial attempt does NOT appear to fix issue on MacBook Pro

sezero commented 1 year ago

Initial attempt does NOT appear to fix issue on MacBook Pro

I assume you tested the second patch, yes?

I tested it on i686 linux (x11, gnome-2) by running qs, loading a game, and doing an Alt-Tab while the game is alive.

I don't have an M1 mac, therefore I can't test and I'm out of ideas.

timbergeron commented 1 year ago

Spike added this and it worked: https://github.com/timbergeron/QSS-M/commit/c0efc3bc9a302ab28803b896f68d4ad74772b0c0

sezero commented 1 year ago

Fixed, thanks