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

Map function keys independently of key group #1598

Closed
wants to merge 3 commits into from

Conversation

ZenulAbidin
Copy link
Contributor

XkbKeycodeToKeysym in function translateKeyCode in x11_init.c is returning keySym value of -1 when called with the key's scancode. This causes all function and numpad keys to be mapped to GLFW_KEY_UNKNOWN.

Printable keys register correctly because they are mapped explicitly, so I mapped the other keys like that too. There is no reason to use translateKeyCode now.

@ZenulAbidin
Copy link
Contributor Author

Also, this bug is not present in the latest release.

@elmindreda elmindreda added the X11 label Dec 19, 2019
@elmindreda
Copy link
Member

Also, this bug is not present in the latest release.

Do you mean that this bug is present in master but not in 3.3?

@elmindreda elmindreda added bug Bug reports and bugfix pull requests cannot reproduce Bugs that have failed verification labels Dec 23, 2019
@elmindreda elmindreda changed the title X11: Fix bug that mapped all function keys to GLFW_KEY_UNKNOWN Fix bug that mapped all function keys to GLFW_KEY_UNKNOWN Dec 23, 2019
@elmindreda elmindreda self-assigned this Dec 23, 2019
@elmindreda elmindreda added this to In progress in Queue Dec 23, 2019
@elmindreda elmindreda removed this from In progress in Queue Jan 2, 2020
@elmindreda elmindreda added the waiting for reply Issues blocked waiting for information label Jan 2, 2020
@ZenulAbidin
Copy link
Contributor Author

After running git bisect, I have discovered that this regression was introduced in commit b3eb6dd. The problem is triggered here:

glfw/src/x11_init.c

Lines 670 to 676 in fe57e3c

_glfw.x11.xkb.group = 0;
XkbStateRec state;
if (XkbGetState(_glfw.x11.display, XkbUseCoreKbd, &state) == Success)
{
XkbSelectEventDetails(_glfw.x11.display, XkbUseCoreKbd, XkbStateNotify, XkbAllStateComponentsMask, XkbGroupStateMask);
_glfw.x11.xkb.group = (unsigned int)state.group;
}

This is related to issue #1462, because I have an English QWERTY keyboard layout and also an Arabic QWERTY layout on my system (this may be why you couldn't reproduce it). The issue is the function keys are only on group 0. My Arabic layout is group 0 while my English layout is group 1. So the function keys only work if I start the program in Arabic layout. If I then switch to English layout while the program is running, it still works, but it's very inconvenient.

This bug is reproducible in master as well as in GLFW 3.3.1.

Over here:

glfw/src/x11_init.c

Lines 256 to 262 in fe57e3c

// Map the key name to a GLFW key code. Note: We only map printable
// keys here, and we use the US keyboard layout. The rest of the
// keys (function keys) are mapped using traditional KeySym
// translations.
if (strcmp(name, "TLDE") == 0) key = GLFW_KEY_GRAVE_ACCENT;
else if (strcmp(name, "AE01") == 0) key = GLFW_KEY_1;
else if (strcmp(name, "AE02") == 0) key = GLFW_KEY_2;

Since it's assigning ASCII keys here and they return the native character in the current keyboard layout, it would be simpler to assign the function keys there too. The function key names I added in my PR were taken from the evdev key-codes list and I tested all of them.

@elmindreda elmindreda removed the waiting for reply Issues blocked waiting for information label Jan 2, 2020
@elmindreda elmindreda added this to the 3.3.2 milestone Jan 2, 2020
@ZenulAbidin ZenulAbidin changed the title Fix bug that mapped all function keys to GLFW_KEY_UNKNOWN Map function keys independently of key group Jan 2, 2020
@elmindreda elmindreda added this to Work queue in Queue Jan 2, 2020
@elmindreda elmindreda added verified Reproduced or otherwise verified bugs and removed cannot reproduce Bugs that have failed verification labels Jan 2, 2020
@elmindreda
Copy link
Member

Successfully reproduced on Cygwin/X running MATE with the default Arabic layout.

@elmindreda
Copy link
Member

This is a good PR but I cannot merge the removal of the non-XKB code as-is because that path can still be chosen if the XKB extension is unavailable, which would leave all keys unknown. We should either keep the whole fallback path or go all-in and require XKB at init.

The x11-xkb-key-names branch is my proposal for the alternative of keeping the fallback path. I also added checking XKB key aliases, which should help some with non-PC and/or non-evdev platforms, though more work is likely needed for obscure platforms.

Do these changes seem reasonable?

@ZenulAbidin
Copy link
Contributor Author

ZenulAbidin commented Apr 5, 2020

@elmindreda

This is a good PR but I cannot merge the removal of the non-XKB code as-is because that path can still be chosen if the XKB extension is unavailable, which would leave all keys unknown.

Good point. We should keep the code that GLFW falls back to if XKB isn't available.

I believe the changes in the x11-xkb-key-names branch are acceptable.

@elmindreda
Copy link
Member

This has been merged as a41a58a. Thank you for the fix and your patience through all the delays.

@elmindreda elmindreda removed this from Work queue in Queue May 14, 2020
elmindreda pushed a commit that referenced this pull request May 19, 2020
This fixes the issue where function keys would be reported as
GLFW_KEY_UNKNOWN if XKB was available and one of the configured keyboard
layouts was Arabic.

This is only part of #1598, because the full patch removed parts of the
fallback path for when XKB is unavailable.

Closes #1598.

(cherry picked from commit a41a58a)
elmindreda added a commit that referenced this pull request May 19, 2020
Related to #1598.

(cherry picked from commit 215a05a)
@combolek
Copy link

combolek commented Nov 10, 2020

b25ee39 is an important bug fix. May I ask that it be released in 3.3.3 so that Linux distributions pick it up?
Thanks

m4ce-w1ndu pushed a commit to m4ce-w1ndu/glfw that referenced this pull request Jul 23, 2022
This fixes the issue where function keys would be reported as
GLFW_KEY_UNKNOWN if XKB was available and one of the configured keyboard
layouts was Arabic.

This is only part of glfw#1598, because the full patch removed parts of the
fallback path for when XKB is unavailable.

Closes glfw#1598.
m4ce-w1ndu pushed a commit to m4ce-w1ndu/glfw that referenced this pull request Jul 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug reports and bugfix pull requests verified Reproduced or otherwise verified bugs X11
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants