Fixed modifier key state setting in GLFW callbacks.

[unpacklo]

On my Linux system, GLFW 3.1.1, the key callback handles modifier keys incorrectly. Pressing the modifier will set the flag true, but releasing it does not. The only way to unset the modifiers is to hit a non-modifier key.

This issue does not seem to be present on my MacBook Pro.

[ocornut]

Hmm, this is odd. Perhaps it should be raised to GLFW first ?

[unpacklo]

I'll try to see if this is a GLFW issue. Most of the time I don't use the sample program myself, since I usually just rebuild our game to check out new features and we have our own handlers for all of these.

[unpacklo]

I talked with one of the glfw maintainers and they pointed out a bug in my original code where hitting the two of the same modifiers (left + right alt, for example) would not be detected correctly.

Furthermore, I discovered that the reason why Mac and Linux behave differently is that they see the modifier differently in the callback function. Using the events sample program from glfw on Mac yields the following sequence:

dalekim1@dales-mbp:/Users/dalekim1/glfw-3.1.1/tests> ./events 
Library initialized
Creating windowed mode window 1 (640x480)
Main loop starting
00000000 to 1 at 0.140: Window focused
00000001 to 1 at 0.179: Cursor left window
00000002 to 1 at 1.888: Key 0x0154 Scancode 0x0038 (LEFT SHIFT) (with shift) was pressed
00000003 to 1 at 2.039: Key 0x0154 Scancode 0x0038 (LEFT SHIFT) (with no mods) was released
00000004 to 1 at 2.559: Key 0x0156 Scancode 0x003a (LEFT ALT) (with alt) was pressed
00000005 to 1 at 2.671: Key 0x0156 Scancode 0x003a (LEFT ALT) (with no mods) was released
00000006 to 1 at 2.927: Key 0x0155 Scancode 0x003b (LEFT CONTROL) (with control) was pressed
00000007 to 1 at 3.079: Key 0x0155 Scancode 0x003b (LEFT CONTROL) (with 
no mods) was released
00000008 to 1 at 3.655: Key 0x0158 Scancode 0x003c (RIGHT SHIFT) (with shift) was pressed
00000009 to 1 at 3.759: Key 0x0158 Scancode 0x003c (RIGHT SHIFT) (with no mods) was released
0000000a to 1 at 3.967: Key 0x015a Scancode 0x003d (RIGHT ALT) (with alt) was pressed
0000000b to 1 at 4.079: Key 0x015a Scancode 0x003d (RIGHT ALT) (with no mods) was released
0000000c to 1 at 4.215: Key 0x0159 Scancode 0x003e (RIGHT CONTROL) (with control) was pressed
0000000d to 1 at 4.287: Key 0x0159 Scancode 0x003e (RIGHT CONTROL) (with no mods) was released
0000000e to 1 at 5.735: Window close

Meanwhile, my Linux system does:

dalekim1@lispy:/home/dalekim1/glfw/tests [(detached from 3.1.1)]> ./events 
Library initialized
Creating windowed mode window 1 (640x480)
Main loop starting
00000000 to 1 at 0.155: Window position: 3 26
00000001 to 1 at 0.156: Window was restored
00000002 to 1 at 0.158: Window refresh
00000003 to 1 at 0.160: Window focused
00000004 to 1 at 2.509: Key 0x0154 Scancode 0x0032 (LEFT SHIFT) (with no mods) was pressed
00000005 to 1 at 2.637: Key 0x0154 Scancode 0x0032 (LEFT SHIFT) (with shift) was released
00000006 to 1 at 3.749: Key 0x0156 Scancode 0x0040 (LEFT ALT) (with no mods) was pressed
00000007 to 1 at 3.869: Key 0x0156 Scancode 0x0040 (LEFT ALT) (with alt) was released
00000008 to 1 at 4.213: Key 0x0155 Scancode 0x0025 (LEFT CONTROL) 
(with no mods) was pressed
00000009 to 1 at 4.357: Key 0x0155 Scancode 0x0025 (LEFT CONTROL) (with control) was released
0000000a to 1 at 5.005: Key 0x0158 Scancode 0x003e (RIGHT SHIFT) (with no mods) was pressed
0000000b to 1 at 5.157: Key 0x0158 Scancode 0x003e (RIGHT SHIFT) (with shift) was released
0000000c to 1 at 5.845: Key 0x015a Scancode 0x006c (RIGHT ALT) (with no mods) was pressed
0000000d to 1 at 5.973: Key 0x015a Scancode 0x006c (RIGHT ALT) (with alt) was released
0000000e to 1 at 6.085: Key 0x0159 Scancode 0x0069 (RIGHT CONTROL) (with no mods) was pressed
0000000f to 1 at 6.181: Key 0x0159 Scancode 0x0069 (RIGHT CONTROL) (with control) was released
00000010 to 1 at 7.697: Window close

As you can see, the modifier is detected only when the key is released on Linux but Mac sees it when pressed. They told me that the modifiers are only valid for the key event and this particular behavior is actually expected.

The suggestion to work around this was to grab the last reported key state by calling glfwGetKey(), which reports whether the key was previously pressed or released, which is what my new commit uses.

[ocornut]

They told me that the modifiers are only valid for the key event and this particular behavior is actually expected.

I'm merging but I still don't understand this. What does "for the key event" means? Is the discussion on their github? (couldn't find it)

[extrawurst]

i am not convinced blindly merging this was a good idea ,)

[ocornut]

Well, it is a workaround for how GLFW functions. I don't understand why GLFW would provide mods this way.

We have just changed this:

io.KeyCtrl = (mods & GLFW_MOD_CONTROL) != 0;
io.KeyShift = (mods & GLFW_MOD_SHIFT) != 0;
io.KeyAlt = (mods & GLFW_MOD_ALT) != 0;

To that:

(void)mods; // Modifiers are not reliable across systems
io.KeyCtrl = glfwGetKey(window, GLFW_KEY_LEFT_CONTROL) == GLFW_PRESS || glfwGetKey(window, GLFW_KEY_RIGHT_CONTROL) == GLFW_PRESS;
io.KeyShift = glfwGetKey(window, GLFW_KEY_LEFT_SHIFT) == GLFW_PRESS || glfwGetKey(window, GLFW_KEY_RIGHT_SHIFT) == GLFW_PRESS;
io.KeyAlt = glfwGetKey(window, GLFW_KEY_LEFT_ALT) == GLFW_PRESS || glfwGetKey(window, GLFW_KEY_RIGHT_ALT) == GLFW_PRESS;

I must prefer the early obviously, but it's not a big deal. Still, I would like to undersstand the behavior.

[unpacklo]

I spoke with dreda directly on their IRC channel yesterday, so you won't find anything on their GitHub. I still suspect something is amiss in glfw because my understanding of the mods parameter is that it's only provided as extra information to go along with a particular key event, and should not be used to directly set any key state, but the mods parameter behaves in a way that's not consistent and in some cases, not intuitive.

For example, I can use it to know if a mod key was pressed/released when a non-modifier key is pressed/released (such as "A"), but I should not use it to try to detect if a mod key was pressed or released with this mechanism since the actual press/release of the modifier key generates its own event. If you want to know whether a key was pressed or released, you should use its key callback directly (GLFW_KEY_*) instead. But looking at the events output from my Mac and Linux machines, you can see how the reversed reporting of the mods parameter makes this really inconsistent and propagates bugs into user applications.

[unpacklo]

Sorry, that last paragraph was really bad. The distinction I was trying to make is that you should not try to detect state changes of a mod key through the mods parameter, which is basically what we were trying to do.

We're interested in knowing when exactly the modifier keys are released or pressed which, apparently, can only be known by handling the specific GLFW_KEY_* event and checking for its pressed/released. But if mods were simply reported like the Mac does across all systems, then I think we could use the old callback code and everything would be fine. Since Linux does not report it in this way and tells us a mod key was pressed at the same time it was released, our code breaks.

An alternative might be:

io.KeyCtrl = io.KeysDown[GLFW_KEY_LEFT_CONTROL] || io.KeysDown[GLFW_KEY_RIGHT_CONTROL];
io.KeyShift = io.KeysDown[GLFW_KEY_LEFT_SHIFT] || io.KeysDown[GLFW_KEY_RIGHT_SHIFT];
io.KeyAlt = io.KeysDown[GLFW_KEY_LEFT_ALT] || io.KeysDown[GLFW_KEY_RIGHT_ALT];

[ocornut]

An alternative might be:

You mean inside ImGui_ImplGlfw_NewFrame() ? Yeah maybe that's shorter and perhaps match closer than the end-user would do with their own engine. If you don't mind double checking under Mac/Linux (no hurry) to put that under the " // Setup inputs " block I'll push that change. Thanks!

[unpacklo]

I was thinking just inside the key callback, right now I have:

void ImGui_ImplGlfwGL3_KeyCallback(GLFWwindow*, int key, int, int action, int mods)
{
    ImGuiIO& io = ImGui::GetIO();
    if (action == GLFW_PRESS)
        io.KeysDown[key] = true;
    if (action == GLFW_RELEASE)
        io.KeysDown[key] = false;

    (void)mods; // Modifiers are not reliable across systems                                                                                                                                                                                                                    
    io.KeyCtrl = io.KeysDown[GLFW_KEY_LEFT_CONTROL] || io.KeysDown[GLFW_KEY_RIGHT_CONTROL];
    io.KeyShift = io.KeysDown[GLFW_KEY_LEFT_SHIFT] || io.KeysDown[GLFW_KEY_RIGHT_SHIFT];
    io.KeyAlt = io.KeysDown[GLFW_KEY_LEFT_ALT] || io.KeysDown[GLFW_KEY_RIGHT_ALT];
}

Which is fine on Linux, about to check Mac...

[ocornut]

Yes that should be fine, you are right.

[ocornut]

Thanks!