VulkanSceneGraph icon indicating copy to clipboard operation
VulkanSceneGraph copied to clipboard

Probably incorrect handling of left/right ALT key on Windows

Open Mikalai opened this issue 1 year ago • 1 comments

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):

  • Windows 11

Mikalai avatar May 20 '24 15:05 Mikalai

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

  • VK_LMENU/VK_RMENU over VK_MENU: nice to have (I guess VK_MENU could use the same path as VK_LMENU, ie be mapped to KEY_Alt_L, but need to check that this doesn't generate duplicate events)
  • these keys mapped to KEY_Menu: that's certainly wrong
  • VK_APPS not being mapped at all: also wrong, should be mapped to KEY_Menu

lufriem avatar Jun 04 '24 09:06 lufriem