ImGuiIO modifier key states incorrect under Linux/X11 (glfw)

[mrxz]

When using the ImGui Integration under Linux/X11 the ImGuiIO.KeyCtrl/Shift/Alt/Super states aren't always correct. The modifiers in the key events contain the modifier state just before the event. When pressing the Shift key, for example, this results in the following behaviour:

• Pressing the shift key down results in a key event without the shift modifier, ImGuiIO.KeyShift = false
• Releasing the shift key results in a key event with the shift modifier, ImGuiIO.KeyShift = true

For common shortcuts, like Ctrl+C this doesn't cause too much trouble as the key events for the non-modifier key contain the right modifier state and will result in the ImGuiIO.KeyCtrl state being correct. But when combined with mouse events, the incorrect modifier state causes unexpected behaviour. Holding down a modifier key having no effect and the modifier state 'sticking around' after letting go of a modifier key.

This is likely a regression introduced by switching to ImGui's event based input model. See https://github.com/glfw/glfw/issues/1630 for more details and some possible workarounds (though, sadly, none of the workarounds seem ideal).

[mosra]

Hi, thanks for the detailed report :+1:

So, if I understand correctly, this behavior is specific to GLFW, right? If you would be using the SDL application instead (if it's possible for you to test that), it wouldn't happen?

I'm thinking adding an extra bit of code that fills in also the modifier state if on a key down event for a modifier key could fix this, and make the behavior consistent between OSes and toolkits.

@pezcode, since you did all of the ImGui event work and are familiar with the code, is this something you'd be able to look into? Thank you :)

[mrxz]

So, if I understand correctly, this behavior is specific to GLFW, right? If you would be using the SDL application instead (if it's possible for you to test that), it wouldn't happen?

I just tried it and this behaviour is indeed only present when using GlfwApplication.h and not with Sdl2Application.h.

Filling the modifier state also with the (modifier) key being pressed sounds good. Handling the key release probably becomes a bit trickier. For example, someone could be holding down both Shift keys. After releasing only one of them, the shift modifier should remain set. After also releasing the other shift key the shift modifier should explicitly not be set.

Not to mention the concerns raised in the glfw issue (https://github.com/glfw/glfw/issues/1630) regarding such workarounds:

Problem with the ImGui workaround is it conflates Alt (left alt) and AltGr (right alt) for non-English users and it forcibly overrides sticky keys when it clears the mod bit on a keyup event (this discriminates against users with hand disabilities).

That being said, it would already be a great improvement and at least make modifiers with mouse events usable in this setup.

[mrxz]

Useful reference for how ImGui handles this issue: https://github.com/ocornut/imgui/issues/6034#issuecomment-1369620644

They've been inferring the modifiers from the key state since 2015 onwards. Basically ignoring the modifiers provided by GLFW altogether and deriving it themselves from which modifier keys are in the pressed state.

January last year they accidentally removed this workaround (https://github.com/ocornut/imgui/commit/075576744057573083ea2dff7d8c9b46d12a28bf) and later applied a fix that updated the modifiers based on the (current) key being pressed (https://github.com/ocornut/imgui/commit/1ad8ad623e7ec16f9ffb6b86ddf68bfd8deb43b9). So if, for example, the left control key was pressed, it would include CTRL in the modifiers. With this approach they still faced the issue linked above, and are now back to deriving the modifiers manually from which keys are pressed.

Interestingly they apply this workaround on all platforms, despite it only being necessary under Linux/X11.

Applying the same workaround here in the ImGui integration becomes a tricky as querying the key state with glfwGetKey obviously breaks abstraction. But since the issue is with GLFW, it might make more sense to apply the workaround in GlfwApplication instead?