mosra / magnum

Lightweight and modular C++11 graphics middleware for games and data visualization
https://magnum.graphics/
Other
4.74k stars 439 forks source link

MacOS: GlfwApplication.cpp::currentGlfwModifiers wrongly detects modifiers in mouseMoveEvent #593

Open DavidPeicho opened 1 year ago

DavidPeicho commented 1 year ago

Hi,

It looks like GlfwApplication.cpp::currentGlfwModifiers returns the wrong modifiers after opening the MacOS screenshot panel and cancelling it.

The problem comes from glfwGetKey that will return true at:

    if(glfwGetKey(window, GLFW_KEY_LEFT_SHIFT) ||
       glfwGetKey(window, GLFW_KEY_RIGHT_SHIFT))

Reproduction

Fix

I guess one way to fix it would be to only use the glfwSetKeyCallback method instead of using glfwGetKey

DavidPeicho commented 1 year ago

Definitely a bug with glfw and I don't think there is a way around it: https://github.com/glfw/glfw/issues/2189

mosra commented 1 year ago

Hi!

Hmm, I have a vague feeling that this wasn't limited to just GLFW and macOS, but happened to me with SDL on Linux and other apps as well. So maybe not a GLFW-specific bug but rather a general event handling "wart"?

Is it possible for you to try the same with the SDL application? I wonder how it handles that.

DavidPeicho commented 1 year ago

I can give it a try yes. I will do that tomorrow.

The exact bug though comes from this process:

So I would assume it should be specific to GLFW.

mosra commented 1 year ago

Does the same happen e.g. for Cmd+Space that opens the application launcher?

I tried on Linux (with Alt+F2, which opens the Plasm application launcher) and got a release event for Alt right before the window went out of focus. So that's how it should behave, I guess :)

DavidPeicho commented 1 year ago

Spotlight works fine. I will have to find a SDL example to try with

mosra commented 1 year ago

Easiest could be if you build Magnum with MAGNUM_BUILD_TESTS enabled, then there's a PlatformSdl2ApplicationTest executable that prints a message to the console on each input event (and same with GLFW).

But I kinda suspect SDL will work, so maybe let's just wait for a response from GLFW devs :)

bear24rw commented 8 months ago

Possibly related to: https://github.com/glfw/glfw/issues/1011