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

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

Open
Mikalai opened this issue May 20, 2024 · 1 comment
Open

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

Mikalai opened this issue May 20, 2024 · 1 comment

Comments

@Mikalai
Copy link
Contributor

Mikalai commented May 20, 2024

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
@appcodegen
Copy link
Contributor

appcodegen commented Jun 4, 2024

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

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

No branches or pull requests

2 participants