Esc key press doesn't close vsgviewer on Windows

[Mikalai]

Describe the bug Open vsgviewer, press Esc, window doesn't close. On Fedora vsgviewer is closed when Esc is pressed and seems that CloseHandler is responsible for this.

To Reproduce Steps to reproduce the behavior:

1. Open vsgviewer on Windows
2. Press Esc
3. Application is not closed

Expected behavior Application should be closed on Esc key when CloseHandler is used.

Screenshots Under debugger next happens. closeKey which is KEY_Escape is compared to keyPress.keyBase which is 27 (VK_ESCAPE), thus window is not closed.

Desktop (please complete the following information):

• OS: Windows only
• 11

Additional context Changing keyPress.keyBase to keyPress.keyModified fixed the issue on Windows, though not tested on Fedora again.

[robertosfield]

I haven't had a chance to look at this issue yet, I'll review the Windowing code tomorrow and figure out what the most appropriate change would be. FYI, I only have Linux systems here so rely upon members of the VSG community to help support Windows and macOS.

[Mikalai]

Would it be better to send mails to vsg-users group next time? Hope this can help. During investigation of the issue I've reviewed windows code in Win32_Window.cpp and Win32_Window.h.

Here keySymbol is retrieved from _keyboard->getKeySymbol.

    case WM_KEYDOWN:
    case WM_SYSKEYDOWN:
    {
        vsg::KeySymbol keySymbol, modifiedKeySymbol;
        vsg::KeyModifier keyModifier;
        if (_keyboard->getKeySymbol(wParam, lParam, keySymbol, modifiedKeySymbol, keyModifier))
        {
            int32_t repeatCount = (lParam & 0xffff);
            bufferedEvents.emplace_back(vsg::KeyPressEvent::create(this, event_time, keySymbol, modifiedKeySymbol, keyModifier, repeatCount));
        }
        break;
    }

And in getKeySymbol keySymbol is filled with results from Win API function ::ToAscii function.

// our actual keystroke is what we get after the ::ToAscii call
            char asciiKey[2];
            int32_t numChars = ::ToAscii(static_cast<UINT>(wParam), (lParam>>16)&0xff, keyState, reinterpret_cast<WORD*>(asciiKey), 0);
            if (numChars>0) keySymbol = static_cast<vsg::KeySymbol>(asciiKey[0]);

In case of escape it returns ASCII control code for Esc which is 27. Probably here if result belongs to control codes subset of ASCII it should be mapped once again using _keycodeMap, but this also looks a bit over complicated.

Also if numChars for some reasons is 0 keySymbol will remain uninitialized.

[robertosfield]

As a bit of backstory, I wrote the vsg::KeySymbol and Xcb_Window implementation, and Tomas Hogarth was commissioned to implemented the Win32 side. Unfortunately he's focused on other client work since so he's not contributing to the VSG right now. I don't have much Windows experience so generally have to defer to Windows users for testing and improvements. The KeySymbol enums are based on X11 values as is the OSG equivalents, while not a 1:1 mapping those familiar with the OSG should have a bit of familiarity.

-- Now on to problem in hand:

I have reviewed the Win32_Window.h code KeyboardMap::getKeySymbol(..) and what I think the correct behavior should be is to use the keyboardMap even when using ascii converted symbols in order to key everything consistent with the vsg::KeySymbol - a Escape should always be an Escape etc.

As I'm making modifications somewhat blind I have created a Win32_KeyMapping branch to share my proposed changes. Changes so far are in the commit 18aac291733c7f73bcb8e14353806a8538c1605f.

By checking these changes into this branch I am able to leverage the automated builds that include Windows, so I've already seen a compile error which I'll fix now. Once I've checked in a build fix could you test out the Win32_KeyMapping branch and let me know if this resolves the problem?

https://github.com/vsg-dev/VulkanSceneGraph/commit/18aac291733c7f73bcb8e14353806a8538c1605f

[robertosfield]

@Mikalai I've fixed the compile error - adding a vsg:: was all that was required. Could you test the Win32_KeyMapping branch?

[Mikalai]

@robertosfield Looks good. Now Esc closes window. Checked under debugger also and now keyPress has this values

[robertosfield]

Excellent, thanks for the testing. I'll merged with VSG master.