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

Wrong ImGuiKey keydown indexes reported from Win32 backend for some keys when using UK keyboard with UK keyboard layout #7201

Open
tom-seddon opened this issue Jan 6, 2024 · 13 comments

Comments

@tom-seddon
Copy link
Contributor

Version/Branch of Dear ImGui:

278cf1a

Back-ends:

imgui_impl_win32.cpp + imgui_impl_dx11.cpp

Compiler, OS:

Windows 10 (Version 22H2 (OS Build 19045.3803)); Visual Studio 2019 (Microsoft (R) C/C++ Optimizing Compiler Version 19.29.30153 for x64)

Full config/build information:

Dear ImGui 1.90.1 WIP (19002)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=199711
define: _WIN32
define: _WIN64
define: _MSC_VER=1929
define: _MSVC_LANG=201402
--------------------------------
io.BackendPlatformName: imgui_impl_win32
io.BackendRendererName: imgui_impl_dx11
io.ConfigFlags: 0x00000003
 NavEnableKeyboard
 NavEnableGamepad
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x0000000E
 HasMouseCursors
 HasSetMousePos
 RendererHasVtxOffset
--------------------------------
io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,64
io.DisplaySize: 1264.00,761.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 1.00
style.FramePadding: 4.00,3.00
style.FrameRounding: 0.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 8.00,4.00
style.ItemInnerSpacing: 4.00,4.00

Details:

My Issue/Question:

As mentioned as part of #7136 - inconsistent results with Win32 backend using UK vs US keyboards.

Reproduction steps:

  1. Attach standard UK-type ISO keyboard with standard UK-type UK keycaps (see https://en.wikipedia.org/wiki/British_and_American_keyboards#/media/File:KB_United_Kingdom.svg for the expected physical keyboard layout), and select United Kingdom keyboard layout.
  2. Build and run example_win32_directx11
  3. Visit Tools > Metrics in the demo, and expand the Inputs section

Now press some keys. The Chars queue section gets filled with the right chars, so text widget input generally works as expected, and most of the time that's the main thing you want, and so most of the time everything does work fine.

But note the following erroneous correspondence between UK keycaps and the keys that Dear ImGui reports in the 'Keys down' section.

Unshifted Keycap Shifted Keycap Dear ImGui Key reported VK_ code
` ~ VK_OEM_8
' @ GraveAccent VK_OEM_3
# ~ Apostrophe VK_OEM_7

If you're trying to use the key down info for something, then relating this to the actual keys is more difficult than it ought to be. The wrong keys are reported as GraveAccent and Apostrophe and there's a key that doesn't correspond to any Dear ImGui enum at all.

If you attach a standard US-type ANSI keyboard with standard US-type US keycaps (see https://en.wikipedia.org/wiki/British_and_American_keyboards#/media/File:KB_United_States-NoAltGr.svg for the expected physical layout), and select US keyboard layout, the reported keys down all as expected.

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

// Here's some code anyone can copy and paste to reproduce your issue
ImGui::Begin("Example Bug");
MoreCodeToExplainMyIssue();
ImGui::End();
@tom-seddon tom-seddon changed the title Keyboard layout input issue with Win32 backend Wrong ImGuiKey keydown indexes reported from Win32 backend for some keys when using UK keyboard with UK keyboard layout Jan 6, 2024
@ocornut
Copy link
Owner

ocornut commented Jan 6, 2024

Hello Tom,
Isn't that essentially the same issue as #7136 ?
Do you have a suggested fix or possible improvement ?

@tom-seddon
Copy link
Contributor Author

tom-seddon commented Jan 6, 2024

I thought I'd raise it as a separate issue, as it's specifically an issue with the Dear ImGui Win32 backend, in that incorrect keys are reported. #7136 is about there being not enough values in the ImGuiKey enum to cover every possible key.

Looking at this some more, with a potential PR in mind: I have to admit, I don't actually have any suggestions for a fix. The standard Windows keyboard layouts are a bit random in terms of which keys are mapped to VK_OEM_1...VK_OEM_8. For example, the key at the top left of the main area of the keyboard, marked ` unshifted on standard US and UK keyboards, varies quite a lot with the layouts I looked at:

Layout VK_ code
United Kingdom VK_OEM_8
US VK_OEM_3
French VK_OEM_7
Italian VK_OEM_5
German VK_OEM_5 (dead key)
Spanish VK_OEM_5

You can get an ASCII char from VK_ code, so in principle you can work through every vkey code (there are only 256) and build up a table that. But now you have to decide what to do if you get something unexpected - for example, French seems to have a specific ! key, not obviously mapping to any ImGuiKey enum value, but no specific [ key to map to ImGuiKey_LeftBracket.

Maybe the fix is just to say that the enum values are named for the physical keys on a US-layout keyboard specifically, and that the mapping to ImGuiKey might simply be wrong, in some keyboard layout-dependent fashion, even if there's no obvious reason for it. I guess the comment in the code about these being scancodes kind of hints at this being the case.

(By "there's no obvious reason for it", what I mean is that you press a key that has an obvious corresponding entry in the ImGuiKey enum, for example ' on a UK keyboard, and it comes out as some value other than ImGuiKey_Apostrophe.)

This line of thought gives me an idea for a fix for #7136, something I will put together now.

@tom-seddon
Copy link
Contributor Author

I pushed a 1-line PR with an additional comment, that hopefully explains the scope of the ImGuiKey enum and sets caller expectations. The wording is just a suggestion.

@ocornut
Copy link
Owner

ocornut commented Feb 12, 2024

It's too difficult for me to make sense of #7136, #7201, #7206, #7306 individually.
I don't think I can merge #7206 without seeing the bigger picture.

I guess we should methodically collect, for variety of keyboard layout, windowing libs and OS:

  • scancode AND keycode values along with their define name
  • along with keycaps and physical position
  • along with ImGuiKey as currently reported.

In a same format. I don't know otherwise how to proceed as it'll be too overwhelming for me to take a decision without seeing a master table of this kind. You did some of it at #7136 (comment)
I'll see if I have time to come up with a template for us to begin gathering this info.

Some visual ref, US vs UK (https://s-basak.medium.com/why-it-is-better-to-use-us-keyboard-even-if-you-live-in-uk-1a2703bc93dc)
jiRVdOA9x3B44BewaLnOfQ

FR
KB_France svg

EDIT Better visual references at https://www.farah.cl/Keyboardery/A-Visual-Comparison-of-Different-National-Layouts and https://www.farah.cl/Keyboardery/Interactive-Comparator-of-Different-National-Layouts/

@ocornut
Copy link
Owner

ocornut commented Feb 12, 2024

Part of the difficulty is we expect translated keys because we want to maximize meaningful coverage by shortcuts (#456).
Because of that our keys are mostly not based on scancodes but keycodes, so there is the possibility of gaps.

@ocornut
Copy link
Owner

ocornut commented Feb 12, 2024

Gathering Data

I started working on a big sheet to gather info for the "problematic" keys:
https://docs.google.com/spreadsheets/d/1TwAHusBpxf_avmk03FUSrni17RP8qXmbJeDzs5fs0nY/edit?usp=sharing

Note the confusing "Physical KB Layout" vs "Input Layout" which is because I tested French on a US keyboard.
As soon as I can double check result on actual french keyboard I may simplify this further.

@ocornut
Copy link
Owner

ocornut commented Feb 13, 2024

I have update the sheet above with multitude of entries for:

  • Windows
  • SDL 2, GLFW 3.2, Win32 API
  • US and UK keyboards
  • US, UK, FR layouts.

It shows a bunch of holes but I believe we may need more data to decide what to do.

I can't get SDL to emit scancode SDL_SCANCODE_NONUSHASH.

We will need more entries.

Add to backend:

#include "imgui_internal.h"

in ImGui_ImplGlfw_KeyCallback():

    if (imgui_key == ImGuiKey_None)
        IMGUI_DEBUG_LOG("Key Scancode:%d Keycode:%d = *UNHANDLED\n", scancode, keycode);
    else
        IMGUI_DEBUG_LOG("Key Scancode:%d Keycode:%d = '%s'\n", scancode, keycode, ImGui::GetKeyName(imgui_key));

in ImGui_ImplSDL2_KeycodeToImGuiKey(), `SDL_KEYDOWN/SDL_KEYUP' handler:

            if (key == ImGuiKey_None)
                IMGUI_DEBUG_LOG("Key: Scancode=%d, Keycode=%d (%08X) *UNHANDLED*\n", event->key.keysym.scancode, event->key.keysym.sym, event->key.keysym.sym);
            else
                IMGUI_DEBUG_LOG("Key: Scancode=%d, Keycode=%d (%08X) = '%s'\n", event->key.keysym.scancode, event->key.keysym.sym, event->key.keysym.sym, ImGui::GetKeyName(key));

in ImGui_ImplWin32_WndProcHandler():

            if (key == ImGuiKey_None)
                IMGUI_DEBUG_LOG("Key: Scancode = %d, VK = %04X = *UNHANDLED*\n", scancode, vk);
            else
                IMGUI_DEBUG_LOG("Key: Scancode = %d, VK = %04X = '%s'\n", scancode, vk, ImGui::GetKeyName(key));

Then view in console or Demo->Tools->Debug Log.

tom-seddon added a commit to tom-seddon/imgui that referenced this issue Feb 13, 2024
@tom-seddon
Copy link
Contributor Author

tom-seddon commented Feb 13, 2024

Added some macOS ones here: https://docs.google.com/spreadsheets/d/1c0RXmB9Qf7skGP7zSwfTp-d006wuWYd21Xa1Bs25hPQ/edit?usp=sharing - I explicitly state UK PC/US PC as keyboard type as the Apple ones sometimes have different keycaps.

(I think US PC and US Apple might be the same, but I don't want to bet on it.)

You can see the edits I made in my branch here: https://github.com/tom-seddon/imgui/tree/tom/issue7201

(I'm less sure about macOS, but on Windows you can load arbitrary keyboard layouts and query the layout. (Keyboard layouts; MapVirtualKeyExW) So in principle it might be possible to run through all the standard layouts and get some of this info that way?)

@ocornut
Copy link
Owner

ocornut commented Feb 14, 2024

Thank you Tom. Copied them back in main sheet.

Will investigate when I have some courage to dig through that :D

(PS: I also took the liberty to merge your makefile fix)

@ocornut
Copy link
Owner

ocornut commented Feb 14, 2024

@GamingMinds-DanielC assuming you have access to a physical German keyboard, do you think you could contribute gathering some extra data points? It's rather tedious to do it thoroughly, to be honest, but I think only with a nice data set we can see the big picture and decide how to move forward. If you can spot other keys that are not covered as ImGuiKey or widely misleadingly/incorrect they are worth recording too. Thank you!

@GamingMinds-DanielC
Copy link
Contributor

@GamingMinds-DanielC assuming you have access to a physical German keyboard, do you think you could contribute gathering some extra data points?

Sorry, was visiting family last week and didn't see this before today. Here is some quick data based on 1.90.2 WIP.

OS: Windows
Backend: Win32
Physical KB Layout: DE (locale name DE stands for German)
Input Layout: DE-DE

I don't know what "expected markings" would mean that differs from the physical markings, so these two are the same in this list. First character each time is when pressed without shift, second with shift and a potential third character can be reached when pressed with the "Alt Gr" key. Unlisted keys are as expected.

 physical markings | expected markings | scancode |      keycode       |   ImGuiKey
-------------------+-------------------+----------+--------------------+--------------
                ^° |                ^° |       41 | 0xDC =    VK_OEM_5 | Backslash
               ß?\ |               ß?\ |       12 | 0xDB =    VK_OEM_4 | LeftBracket
                ´` |                ´` |       13 | 0xDD =    VK_OEM_6 | RightBracket
                üÜ |                üÜ |       26 | 0xBA =    VK_OEM_1 | Semicolon
               +*~ |               +*~ |       27 | 0xBB = VK_OEM_PLUS | Equal
                öÖ |                öÖ |       39 | 0xC0 =    VK_OEM_3 | GraveAccent
                äÄ |                äÄ |       40 | 0xDE =    VK_OEM_7 | Apostrophe
                #' |                #' |       43 | 0xBF =    VK_OEM_2 | Slash
               <>| |               <>| |       86 | 0xE2 =  VK_OEM_102 | *UNHANDLED*

Small suggestion: if you want to make data gathering easier for basically anyone, you could put the required changes into a branch "testing/gather_keycodes" or something like it, possibly even with log window already activated in the example code.

@ocornut
Copy link
Owner

ocornut commented Feb 20, 2024

Thanks, I added yours to: https://docs.google.com/spreadsheets/d/1TwAHusBpxf_avmk03FUSrni17RP8qXmbJeDzs5fs0nY/edit#gid=0
SDL and GLFW ones would be useful if you can.

The "expected marking" column is a handy reference for the case where the input layout doesn't match the natural keyboard layout.

Small suggestion: if you want to make data gathering easier for basically anyone, you could put the required changes into a branch "testing/gather_keycodes" or something like it, possibly even with log window already activated in the example code.

Gathering that data is quite error-prone, so to be honest I don't think it's a bad idea to add a little bit of entry barrier to avoid passer-by making flaky data submission.

Btw found this helpful link https://www.farah.cl/Keyboardery/A-Visual-Comparison-of-Different-National-Layouts and https://www.farah.cl/Keyboardery/Interactive-Comparator-of-Different-National-Layouts/

@GamingMinds-DanielC
Copy link
Contributor

I never used those platforms before, but I got SDL to work. Based on a fresh ImGui 1.90.4 WIP and SDL 2.30.0:

OS: Windows
Backend: Win32
Physical KB Layout: DE (locale name DE stands for German)
Input Layout: DE-DE

 physical markings | expected markings |              scancode             |         keycode         |  ImGuiKey
-------------------+-------------------+-----------------------------------+-------------------------+-------------
                ^° |                ^° |  53 = SDL_SCANCODE_GRAVE          | 0x5E =  94 = SDLK_CARET | *UNHANDLED*
               ß?\ |               ß?\ |  45 = SDL_SCANCODE_MINUS          | 0xDF = 223 = ???        | *UNHANDLED*
                ´` |                ´` |  46 = SDL_SCANCODE_EQUALS         | 0xB4 = 180 = ???        | *UNHANDLED*
                üÜ |                üÜ |  47 = SDL_SCANCODE_LEFTBRACKET    | 0xFC = 252 = ???        | *UNHANDLED*
               +*~ |               +*~ |  48 = SDL_SCANCODE_RIGHTBRACKET   | 0x2B =  43 = SDLK_PLUS  | *UNHANDLED*
                öÖ |                öÖ |  51 = SDL_SCANCODE_SEMICOLON      | 0xF6 = 246 = ???        | *UNHANDLED*
                äÄ |                äÄ |  52 = SDL_SCANCODE_APOSTROPHE     | 0xE4 = 228 = ???        | *UNHANDLED*
                #' |                #' |  49 = SDL_SCANCODE_BACKSLASH      | 0x23 =  35 = SDLK_HASH  | *UNHANDLED*
               <>| |               <>| | 100 = SDL_SCANCODE_NONUSBACKSLASH | 0x3C =  60 = SDLK_LESS  | *UNHANDLED*

The problematic keys are the same as with Win32. All other keys are as expected. I also attempted GLFW, but it looks like I have to build the library myself to get it running. Maybe I'll try it again when I have a bit more time.

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

No branches or pull requests

4 participants
@tom-seddon @ocornut @GamingMinds-DanielC and others