Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backends: Win32: Fix ImGuiMod_Super being mapped to VK_APPS #7768

Closed
wants to merge 1 commit into from

Conversation

Aemony
Copy link
Contributor

@Aemony Aemony commented Jul 4, 2024

The ImGui_ImplWin32_UpdateKeyModifiers() function maps ImGuiMod_Super to VK_APPS, the "Application" key located between the Right Windows (Super) and Right Control keys on the keyboard, see https://conemu.github.io/en/AppsKey.html

This means that when using ImGui::GetIO().KeySuper to try to get the down state of the VK_RWIN or VK_LWIN keys, it'll always return FALSE when either of those keys are held down, and only return TRUE when VK_APPS is held down.

Changing io.AddKeyEvent(ImGuiMod_Super, IsVkDown(VK_APPS)); to io.AddKeyEvent(ImGuiMod_Super, IsVkDown(VK_LWIN) || IsVkDown(VK_RWIN)); solves the issue and properly maps KeySuper to the two Super keys.

The `ImGui_ImplWin32_UpdateKeyModifiers()` function maps `ImGuiMod_Super` to `VK_APPS`, the "Application" key located between the Right Windows (Super) and Right Control keys on the keyboard, see https://conemu.github.io/en/AppsKey.html

This means that when using `ImGui::GetIO().KeySuper` to try to get the down state of the `VK_RWIN` or `VK_LWIN` keys, it'll always return FALSE when either of those keys are held down, and only return TRUE when `VK_APPS` is held down.

Changing `io.AddKeyEvent(ImGuiMod_Super, IsVkDown(VK_APPS));` to `io.AddKeyEvent(ImGuiMod_Super, IsVkDown(VK_LWIN) || IsVkDown(VK_RWIN));` solves the issue and properly maps `KeySuper` to the two Super keys.
@ocornut ocornut changed the title Fix ImGuiMod_Super being mapped to VK_APPS Backends: Win32: Fix ImGuiMod_Super being mapped to VK_APPS Jul 8, 2024
@ocornut
Copy link
Owner

ocornut commented Jul 8, 2024

Thanks for the PR.
This change was first introduced by 0755767. I recall we did some back and forth with @thedmd on some of the odd specificity of mods on Win32, but I am surprised there's no commentary about why precisely we started using VK_APPS here...

(Digging results:)
thedmd: in our thread 2022-01-17 you used VK_WIN in pseudo-code, i pointed out it didn't exist, replaced with VK_LWIN || VK_RWIN at 2:44 pm, and somehow at 3:08 pm i pushed commit above using VK_APPS, and nowhere in our conversation this is mentioned... So... I am confused about what happened.

Any idea?

Either way, no code seems to use ModSuper on Win32 since I'm not even sure it is legal to do as it triggers Windows behaviors anyhow.
I think the PR is right, but I am curious to understand how your stumbled on this, and how/if you can use the key in some meaningful manner.

@ocornut
Copy link
Owner

ocornut commented Jul 8, 2024

Pushed as 9504068, thanks!
Commentary/hypothesis on the above mistake still welcome :)
(Also note the key handler already translated VK_APPS to ImGuiKey_Menu)

ocornut pushed a commit that referenced this pull request Jul 8, 2024
… of VK_LWIN||VK_RWIN (#7768, #4858, #2622)

Amend 0755767

The `ImGui_ImplWin32_UpdateKeyModifiers()` function maps `ImGuiMod_Super` to `VK_APPS`, the "Application" key located between the Right Windows (Super) and Right Control keys on the keyboard, see https://conemu.github.io/en/AppsKey.html

This means that when using `ImGui::GetIO().KeySuper` to try to get the down state of the `VK_RWIN` or `VK_LWIN` keys, it'll always return FALSE when either of those keys are held down, and only return TRUE when `VK_APPS` is held down.
@ocornut ocornut closed this Jul 8, 2024
@Aemony
Copy link
Contributor Author

Aemony commented Jul 8, 2024

Hi.

It's possible the mistake was made due to the factors surrounding VK_APPS and VK_LWIN/VK_RWIN. According to Wikipedia, all three keys were introduced on keyboards at the same time, and it's a bit surprising how a unified VK_WIN does not exist like it does for other modifier keys. So it's a weird outlier made worse by the fact that not all keyboards even has a VK_APPS key on them... Or they might lack a right VK_RWIN Windows key and only have a VK_APPS key in its place on the right side of the spacebar! It might have also been due to how the unified VK_MENU technically refers to the Alt modifier key, and not this Apps key, which is referred to as the "Menu key" in various places.

I ran into a similar key mistake in another project time where VK_PRINT was mistaken for the PrintScreen key, when its really the VK_SNAPSHOT key. Apparently VK_PRINT refers to some long-obsolete PRINT key of another kind, though luckily that's not a mistake made in Dear ImGui.

So VK_MENU is not the Menu key but the Alt key. VK_APPS is not the typical Windows ("Application" key) but the Apps/Menu key. VK_PRINT is not the PrintScreen key but the Print key... 🤣

Microsoft does not make these things simple! 😅


As for how I stumbled upon it, it's because our project makes use of global hotkeys registered through Win32's RegisterHotKey function. In that function, the Windows modifier key can be used for global hotkeys as long as no other app or the OS itself currently makes use of the exact key combination. Making use of Windows+Ctrl+Shift as a modifier base for global hotkeys is a surprisingly safe way of ensuring the availability of the desired hotkey.

Right now this is only implemented in the early experimental Special K Image Viewer (SKIV) tool where it is mapped to capture a screenshot of the desktop or a region on it, but it'll eventually also be implemented in our Special K Injection Frontend (SKIF) tool where we also make use of global hotkeys mapped to the Windows key (in this case to trigger HDR support in Windows on a per-display basis).

The Windows key is also technically safe to use as a modifier for a few normal key combos in games, though it's hardly surprising that nobody uses it since any mistake would just result in the OS stealing focus from the game. As Dear ImGui sees more and more use among more classic desktop applications, I wouldn't be surprised to see the use of ImGuiMod_Super increase as a result as well due to users wanting to bind global hotkeys to the modifier.

Thanks for the quick merge, and cheers!

@thedmd
Copy link
Contributor

thedmd commented Jul 8, 2024

Any idea?

To be honest I'm equally surprised to see VK_APPS as you're. I cannot recall ever using it. I think using VK_LWIN and VK_RWIN is the right call.
😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants