Hey Jorgen, when you have a chance, can you take a quick look at this?

Thanks!

I can reproduce being unable to move the window and that mouse clicks are lost, but I don't know what you mean by "mouse cursor is frozen." I still see SDL_MOUSEMOTION events.

The issue seems to be that we warp the mouse cursor to the center of the window as soon as we get keyboard focus, even if the user is trying to move the window. Not entirely sure why the button events are lost, but we can investigate that after.

The fix would be to only warp the mouse cursor *after* the window move event is done. I cannot seem to find a way to detect this though - there is a windowWillMove: (which happens when you mousedown on the title bar), and a windowDidMove: (which happens when you mouseup *or* at regular intervals.) Mouse-up events are not dispatched to our window.

While not a great solution, maybe if you give the window focus by trying to move it, we should leave the cursor alone until you move the cursor over the window itself (and not the title bar), at which point we warp it to the center?

Created attachment 1579 [details]
Workaround

Here's somewhat of a workaround patch.  I too could not find a way to definitively detect the end of a window move.  My hack is to delay the key-focus-gained event, wait for a mouse move event (and assume that the move is then done), and then allow the key-focus-gained event to happen.  This isn't pretty, and doesn't give me warm fuzzies, but it seems to work "OK".

Note: In the patch, ASL_SDL is simply a macro we define when building our SDL lib, to easily identify our SDL mods.

(In reply to Tim McDaniel from comment #4)
> Created attachment 1579 [details]
> Workaround
> 
> Here's somewhat of a workaround patch.  I too could not find a way to
> definitively detect the end of a window move.  My hack is to delay the
> key-focus-gained event, wait for a mouse move event (and assume that the
> move is then done), and then allow the key-focus-gained event to happen. 
> This isn't pretty, and doesn't give me warm fuzzies, but it seems to work
> "OK".
> 
> Note: In the patch, ASL_SDL is simply a macro we define when building our
> SDL lib, to easily identify our SDL mods.

Here's a slightly cleaner fix for the same issue - we use the "mouse up" event from -[NSWindow sendEvent:] to approximate when the move is finished. Do you want to test this revision and see if it resolves the issue for you too, Tim?

https://hg.libsdl.org/SDL/rev/028ed8da2524

Great fix, thanks.  One case it does not handle well is this:

* Key-focused window, relative mouse mode.
* Cmd-Tab away from window.
* App responds to SDL_WINDOWEVENT_FOCUS_LOST event by turning off relative mouse mode.
* Try to move the window with the mouse.
* App responds to the immediate SDL_WINDOWEVENT_FOCUS_GAINED event by turning on relative mouse mode.
* Window stops moving, you can't move the window any more, mouse cursor is stuck until you let go of the mouse button.
* All is well after that.

I don't know if it's critical that relative mouse mode be on before Cmd-Tabbing away from the window and turning relative mode off in response to the focus lost event.  It's probably just the act of turning on relative mode in response to the focus gained event that causes the problem.  The thing that my patch did to avoid this is to delay the focus gained event till after the window move was finished.

(In reply to Tim McDaniel from comment #6)
> Great fix, thanks.  One case it does not handle well is this:
> 
> * Key-focused window, relative mouse mode.
> * Cmd-Tab away from window.
> * App responds to SDL_WINDOWEVENT_FOCUS_LOST event by turning off relative
> mouse mode.
> * Try to move the window with the mouse.
> * App responds to the immediate SDL_WINDOWEVENT_FOCUS_GAINED event by
> turning on relative mouse mode.
> * Window stops moving, you can't move the window any more, mouse cursor is
> stuck until you let go of the mouse button.
> * All is well after that.
> 
> I don't know if it's critical that relative mouse mode be on before
> Cmd-Tabbing away from the window and turning relative mode off in response
> to the focus lost event.  It's probably just the act of turning on relative
> mode in response to the focus gained event that causes the problem.  The
> thing that my patch did to avoid this is to delay the focus gained event
> till after the window move was finished.

When you say "app responds", you mean that the app (and not SDL) handles SDL_WINDOWEVENT_FOCUS_GAINED by calling SDL_SetMouseRelativeMode(SDL_TRUE)? That's not needed - you should just leave relative mode on when the app loses focus. That should work well on all platforms. There might've been a bug in the past that required this, but SDL 2.x should have no such problems.

(In reply to Jørgen Tjernø from comment #7)
> When you say "app responds", you mean that the app (and not SDL) handles
> SDL_WINDOWEVENT_FOCUS_GAINED by calling SDL_SetMouseRelativeMode(SDL_TRUE)?
> That's not needed - you should just leave relative mode on when the app
> loses focus. That should work well on all platforms. There might've been a
> bug in the past that required this, but SDL 2.x should have no such problems.

Yes, I mean the app, not SDL.

Unfortunately, in our case, I can't really prevent the SDL_SetMouseRelativeMode(SDL_TRUE) in response to SDL_WINDOWEVENT_FOCUS_GAINED.  The focus-gained event gets passed on to other code layers whose behavior shouldn't be modified.  I can certainly live with your current fix, it gets 90% there anyway.  ;-)  I'll just have to modify it a bit.

Created attachment 1581 [details]
Don't apply relative mode when moving

(In reply to Tim McDaniel from comment #8)
> (In reply to Jørgen Tjernø from comment #7)
> > When you say "app responds", you mean that the app (and not SDL) handles
> > SDL_WINDOWEVENT_FOCUS_GAINED by calling SDL_SetMouseRelativeMode(SDL_TRUE)?
> > That's not needed - you should just leave relative mode on when the app
> > loses focus. That should work well on all platforms. There might've been a
> > bug in the past that required this, but SDL 2.x should have no such problems.
> 
> Yes, I mean the app, not SDL.
> 
> Unfortunately, in our case, I can't really prevent the
> SDL_SetMouseRelativeMode(SDL_TRUE) in response to
> SDL_WINDOWEVENT_FOCUS_GAINED.  The focus-gained event gets passed on to
> other code layers whose behavior shouldn't be modified.  I can certainly
> live with your current fix, it gets 90% there anyway.  ;-)  I'll just have
> to modify it a bit.

If you try the latest patch attached to this bug, does that solve it?

Basically it makes Cocoa_SetRelativeMouseMode() not call CGAssociateMouseAndMouseCursorPosition() if we're in the process of moving a window, with the assumption that the mouse relative mode will be applied once the window finishes moving / gets focus. We could even do the early out if SDL_GetMouseFocus() returns NULL.

(In reply to Jørgen Tjernø from comment #10)
> If you try the latest patch attached to this bug, does that solve it?
> 
> Basically it makes Cocoa_SetRelativeMouseMode() not call
> CGAssociateMouseAndMouseCursorPosition() if we're in the process of moving a
> window, with the assumption that the mouse relative mode will be applied
> once the window finishes moving / gets focus. We could even do the early out
> if SDL_GetMouseFocus() returns NULL.

That patch does work pretty well.

Okay, if there are no more issues with this behavior, I'm considering this bug fixed.

Committed as https://hg.libsdl.org/SDL/rev/acc0dcf38b3d

Thanks for the report & help in fixing it!

(In reply to Jørgen Tjernø from comment #12)
> Okay, if there are no more issues with this behavior, I'm considering this
> bug fixed.
> 
> Committed as https://hg.libsdl.org/SDL/rev/acc0dcf38b3d
> 
> Thanks for the report & help in fixing it!

In that commit, you forgot a value for the second return statement.
Thanks!

Haha, whoops, thanks for the catch. I wonder why buildbot didn't email me.