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.

Sign up for GitHub

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

Jump to bottom

[3.x] Fix non-latin layout scancodes on Linux, adds access to physical scancodes. #46764

Merged
merged 2 commits into from
May 6, 2021
Merged

[3.x] Fix non-latin layout scancodes on Linux, adds access to physical scancodes. #46764

merged 2 commits into from
May 6, 2021

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Mar 7, 2021 •
edited
Loading

Backport of #18020, without breaking changes.

Existing Functions properties are not renamed, new property is physical_scancode.

For the reference, list of changes to existing API (in both cases physical scancode is used only if remapped scancode is 0):

  • InputEventKey::action_match uses physical scancode, if remapped scancode is 0 (Physical key are added to InputMap with scancodes == 0, remapped keys with physical_scancode == 0).
  • On Linux/X11, physical scancode is used in place remapped, if scancode is 0 (not remapped correctly by XLookupString).

Modified test project from 46751: Test.zip

Screenshots: Dvorak: Screenshot 2021-03-07 at 16 45 10 Russian (phonetic) layout: Screenshot 2021-03-07 at 16 45 32 Results are the same on macOS, Linux (Ubuntu 20.10) and Windows.

Fixes #46751 and fixes #46750

@bruvzg bruvzg added this to the 3.2 milestone Mar 7, 2021
@bruvzg bruvzg requested review from a team as code owners March 7, 2021 16:06
@bruvzg bruvzg force-pushed the input_fix_non_latin_and_add_hw_scancodes_3 branch from 43fd802 to 266e731 Compare March 7, 2021 16:22
Faless
Faless reviewed Mar 7, 2021
View reviewed changes
platform/javascript/os_javascript.cpp Outdated
@@ -253,6 +253,7 @@ static Ref<InputEventKey> setup_key_event(const EmscriptenKeyboardEvent *emscrip
ev->set_echo(emscripten_event->repeat);
dom2godot_mod(emscripten_event, ev);
ev->set_scancode(dom_code2godot_scancode(emscripten_event->code, emscripten_event->key));
ev->set_physical_scancode(dom_code2godot_scancode(emscripten_event->code, emscripten_event->key));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is actually "proper" (quoted because we are still talking about the web :( ) implementation for this. Let me do the backport on top of these changes so you could simply add it on top.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See Faless@516eace

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I have pulled JS commit to the PR branch.

Base automatically changed from 3.2 to 3.x March 16, 2021 11:11
@aaronfranke aaronfranke modified the milestones: 3.2, 3.3 Mar 16, 2021
@bruvzg bruvzg changed the title [3.2] Fix non-latin layout scancodes on Linux, adds access to physical scancodes. [3.3] Fix non-latin layout scancodes on Linux, adds access to physical scancodes. Mar 16, 2021
@akien-mga akien-mga modified the milestones: 3.3, 3.4 Mar 26, 2021
@akien-mga akien-mga changed the title [3.3] Fix non-latin layout scancodes on Linux, adds access to physical scancodes. [3.x] Fix non-latin layout scancodes on Linux, adds access to physical scancodes. Mar 26, 2021
bruvzg and others added 2 commits May 6, 2021 23:19
@bruvzg
Add physical_scancode (keyboard layout independent keycodes) to Inp…
dab4cf3 
…utEventKey and InputMap.

Fix non-latin keyboard layout keycodes on Linux/X11 (fallback to physical keycodes).
@Faless @bruvzg
[HTML5] Add support for physical_keycode.
8740e95 
This uses the `event.code` value to retrieve the physical code, while
still using the extra logic to map the unicode value to our keylist,
when computing the `scancode` (supporting ASCII and Latin-1).
@bruvzg bruvzg force-pushed the input_fix_non_latin_and_add_hw_scancodes_3 branch from 516eace to 8740e95 Compare May 6, 2021 20:20
@bruvzg bruvzg mentioned this pull request May 6, 2021
Closed
@akien-mga akien-mga merged commit dc612a7 into godotengine:3.x May 6, 2021
@akien-mga
Copy link
Member

Thanks!

@bruvzg bruvzg deleted the input_fix_non_latin_and_add_hw_scancodes_3 branch May 7, 2021 08:07
@AaronRecord AaronRecord mentioned this pull request May 17, 2021
Open
@bruvzg bruvzg mentioned this pull request May 22, 2021
Merged
@madmiraal madmiraal mentioned this pull request May 24, 2021
Closed
@jitspoe
Copy link
Contributor

jitspoe commented Sep 7, 2021

Does this mean we'll be able to bind the default keys based on key positions instead of the letters of the keys? Would be great for non-qwerty keyboard users!

@Calinou
Copy link
Member

Calinou commented Sep 7, 2021 •
edited
Loading

Does this mean we'll be able to bind the default keys based on key positions instead of the letters of the keys? Would be great for non-qwerty keyboard users!

Yes 🙂

I haven't checked the 3.x version, but there should be a Physical Key option when you add a new event to an input action in the Input Map editor.

@jitspoe
Copy link
Contributor

jitspoe commented Sep 7, 2021

Awesome! I saw there's a physical_key that gets written to the project file, but it seems to always be 0. Maybe it's only supported in Linux at the moment?

@akien-mga
Copy link
Member

@jitspoe Should work on all platforms, but key and physical_key are mutually exclusive. You should click "Physical Key" to add one to the input map (and key will be 0).
Screenshot_20210919_112749

@jitspoe
Copy link
Contributor

jitspoe commented Nov 21, 2021

Is there a way to get the display string from a physical_key? Can't seem to find anything, so I made a proposal here: godotengine/godot-proposals#3571

@mischiefaaron
Copy link

I really appreciate this!! Was really hoping I wouldn't have to wait until 4.0 for this and thankfully that is now the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Faless Faless Faless left review comments

@akien-mga akien-mga akien-mga approved these changes

Assignees
No one assigned
Labels
bug enhancement topic:input
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.
7 participants
@bruvzg @akien-mga @jitspoe @Calinou @mischiefaaron @Faless @aaronfranke