Skip to content

Commit

Permalink
Backends: GLFW: Fix CTRL+A, CTRL+Y, CTRL+Z to match keyboard layout. …
Browse files Browse the repository at this point in the history
…Converting GLFW untranslated keycodes back to translated keycodes. (#456, #2625)
  • Loading branch information
ocornut committed Jan 5, 2022
1 parent 4d023bd commit 100ede5
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 0 deletions.
21 changes: 21 additions & 0 deletions backends/imgui_impl_glfw.cpp
Expand Up @@ -16,6 +16,7 @@

// CHANGELOG
// (minor and older changes stripped away, please see git history for details)
// 2022-01-05: Inputs: Converting GLFW untranslated keycodes back to translated keycodes (in the ImGui_ImplGlfw_KeyCallback() function) in order to match the behavior of every other backend, and facilitate the use of GLFW with lettered-shortcuts API.
// 2021-08-17: *BREAKING CHANGE*: Now using glfwSetWindowFocusCallback() to calling io.AddFocusEvent(). If you called ImGui_ImplGlfw_InitXXX() with install_callbacks = false, you MUST install glfwSetWindowFocusCallback() and forward it to the backend via ImGui_ImplGlfw_WindowFocusCallback().
// 2021-07-29: *BREAKING CHANGE*: Now using glfwSetCursorEnterCallback(). MousePos is correctly reported when the host platform window is hovered but not focused. If you called ImGui_ImplGlfw_InitXXX() with install_callbacks = false, you MUST install glfwSetWindowFocusCallback() callback and forward it to the backend via ImGui_ImplGlfw_CursorEnterCallback().
// 2021-06-29: Reorganized backend to pull data from a single structure to facilitate usage with multiple-contexts (all g_XXXX access changed to bd->XXXX).
Expand Down Expand Up @@ -66,6 +67,7 @@
#else
#define GLFW_HAS_NEW_CURSORS (0)
#endif
#define GLFW_HAS_GET_KEY_NAME (GLFW_VERSION_MAJOR * 1000 + GLFW_VERSION_MINOR * 100 >= 3200) // 3.2+ glfwGetKeyName()

// GLFW data
enum GlfwClientApi
Expand Down Expand Up @@ -147,6 +149,25 @@ void ImGui_ImplGlfw_KeyCallback(GLFWwindow* window, int key, int scancode, int a
if (bd->PrevUserCallbackKey != NULL && window == bd->Window)
bd->PrevUserCallbackKey(window, key, scancode, action, mods);

#if GLFW_HAS_GET_KEY_NAME
// GLFW 3.1+ attempts to "untranslate" keys, which goes the opposite of what every other framework does, making using lettered shortcuts difficult.
// (It had reasons to do so: namely GLFW is/was more likely to be used for WASD-type game controls rather than lettered shortcuts, but IHMO the 3.1 change could have been done differently)
// See https://github.com/glfw/glfw/issues/1502 for details.
// Adding a workaround to undo this (so our keys are translated->untranslated->translated, likely a lossy process).
// This won't cover edge cases but this is at least going to cover common cases.
const char* key_name = glfwGetKeyName(key, scancode);
if (key_name && key_name[0] != 0 && key_name[1] == 0)
{
const char char_names[] = "'-=[]\\,;\'./";

This comment has been minimized.

Copy link
@the-goodies

the-goodies Jan 24, 2022

I think char_names[] has to be equal to: "`-=[]\,;'./";
At 0 index: ` and not '

This comment has been minimized.

Copy link
@ocornut

ocornut Jan 24, 2022

Author Owner

Thank you for spotting this!

const int char_keys[] = { GLFW_KEY_GRAVE_ACCENT, GLFW_KEY_MINUS, GLFW_KEY_EQUAL, GLFW_KEY_LEFT_BRACKET, GLFW_KEY_RIGHT_BRACKET, GLFW_KEY_BACKSLASH, GLFW_KEY_COMMA, GLFW_KEY_SEMICOLON, GLFW_KEY_APOSTROPHE, GLFW_KEY_PERIOD, GLFW_KEY_SLASH, 0 };
IM_ASSERT(IM_ARRAYSIZE(char_names) == IM_ARRAYSIZE(char_keys));
if (key_name[0] >= '0' && key_name[0] <= '9') { key = GLFW_KEY_0 + (key_name[0] - '0'); }
else if (key_name[0] >= 'A' && key_name[0] <= 'Z') { key = GLFW_KEY_A + (key_name[0] - 'A'); }

This comment has been minimized.

Copy link
@cpichard

cpichard Apr 24, 2022

Contributor

should the test handle lower case char as well ? In my application, on a french keyboard, glfwGetKeyName returns 'a' instead of 'A' and the following code does not untranslate the translated key.
Adding else if (key_name[0] >= 'a' && key_name[0] <= 'z') { key = GLFW_KEY_A + (key_name[0] - 'a'); } would fix my issue. Would that be correct ? I can make a PR if so. Thanks

This comment has been minimized.

Copy link
@ocornut

ocornut via email Oct 11, 2022

Author Owner

This comment has been minimized.

Copy link
@cpichard

cpichard via email Oct 12, 2022

Contributor
else if (const char* p = strchr(char_names, key_name[0])) { key = char_keys[p - char_names]; }
}
// if (action == GLFW_PRESS) printf("key %d scancode %d name '%s'\n", key, scancode, key_name);
#endif

ImGuiIO& io = ImGui::GetIO();
if (key >= 0 && key < IM_ARRAYSIZE(io.KeysDown))
{
Expand Down
3 changes: 3 additions & 0 deletions docs/CHANGELOG.txt
Expand Up @@ -64,6 +64,9 @@ Other Changes:
- Platform IME: add ImGuiPlatformImeData::WantVisible, hide IME composition window when not used. (#2589) [@actboy168]
- Platform IME: add ImGuiPlatformImeData::InputLineHeight. (#3113) [@liuliu]
- Platform IME: [windows] call ImmSetCandidateWindow() to position candidate window.
- Backends: GLFW: fix CTRL+A, CTRL+Z, CTRL+Y shortcuts to match user keyboard layout. We are now converting GLFW
untranslated keycodes back to translated keycodes in order to match the behavior of every other backend, and
facilitate the use of GLFW with lettered-shortcuts API. (#456, #2625)
- Backends: OpenGL3: Fixed a buffer overflow in imgui_impl_opengl3_loader.h init (added in 1.86). (#4468, #4830) [@dymk]
It would generally not have noticeable side-effect at runtime but would be detected by runtime checkers.
- Backends: Metal: Added Apple Metal C++ API support. (#4824, #4746) [@luigifcruz]
Expand Down

0 comments on commit 100ede5

Please sign in to comment.