vsg-dev / VulkanSceneGraph

Vulkan & C++17 based Scene Graph Project
http://www.vulkanscenegraph.org
MIT License
1.26k stars 200 forks source link

Probably incorrect handling of left/right ALT key on Windows #1198

Closed Mikalai closed 3 weeks ago

Mikalai commented 3 months ago

Describe the bug I've noticed that on Windows I don't receive vsg::KEY_Alt_L and vsg::KEY_Alt_R for vsg::KeyPressEvent and vsg::KeyReleaseEvent. I've checked Win32_Window.cpp and there is VirtualKeyToKeySymbolMap _vk2vsg which is used for mapping. But for VK_LMENU and VK_RMENU it is mapped to

{VK_LMENU                             ,              KEY_Menu},
{VK_RMENU                             ,              KEY_Menu},

Though according to https://learn.microsoft.com/en-us/previous-versions/windows/embedded/ms927178(v=msdn.10)?redirectedfrom=MSDN

VK_LMENU | 0xA4 | Left ALT
VK_RMENU | 0xA5 | Right ALT

That is why current behavior doesn't look correct. It's not clear why KEY_Menu was used as mapping to vsg::KEY_Alt_L and vsg::KEY_Alt_R seems to be more appropriate.

To Reproduce Check vsg::KeyPressEvent or vsg::KeyReleaseEvent when left or right ALT is pressed on Windows.

Expected behavior Receive vsg::KEY_Alt_L and vsg::KEY_Alt_R when left or right ALT is pressed on Windows.

Desktop (please complete the following information):

appcodegen commented 2 months ago

I agree that it's probably better to use VK_LMENU and VK_RMENU over VK_MENU (which I guess is there for historical reasons only, back when there were no extended 101/102 key keyboards - which didn't feature a 'right' ALT key).

And in any case it being mapped to KEY_Menu seems wrong (instead VK_APPS should use that constant). Currently VK_APPS is mapped to KEY_Undefined (in Win32_Window.cpp), however KeyEvent.h actually states the following

KEY_Menu = 0xFF67, / On Windows, this is VK_APPS, the context-menu key /

So, IMO