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

Cleanup and unify keyboard input. #70052

Merged
merged 1 commit into from
Jan 23, 2023

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Dec 14, 2022

  • Unify keycode values (secondary label printed on a key), remove unused hard-coded Latin-1 codes.
  • Unify IME behavior, add inline composition string display on Windows and X11.
  • Add key_label (primary label printed on a key) value to the key events, and allow mapping actions to the key_label events.
  • Add support for physical keyboard (Bluetooth, USB or Sidecar) handling on iOS.
  • Remove LineEdit/TextEdit selected text when IME input begin.
  • Add support for media key handling on macOS.

KeyboardTranslation

  • Physical keyboard should always have keycode, key_label and physical_keycode set, and unicode set on key press only when it's relevant.
  • IME only sends unicode key down evens, all other keys are ignored while it's active.
  • Virtual keyboards send everything physical do, except physical_keycode.

Supported input modes:

Android iOS Linux/X11 macOS Windows Web
Physical Keyboard Yes 1 Yes 2 Yes Yes Yes Yes 3
Virtual Keyboard Yes Yes No 4 No 4 No 4 No
IME 5 5 Yes (inline) Yes (inline) Yes (inline) No

Fixes #69830
Fixes #54797
Supersede #53682

Footnotes

  1. Tested with Bluetooth and USB-OTG keyboards.

  2. Tested with Bluetooth and USB-OTG keyboards, Smart Connector case keyboard and Sidecar.

  3. key_label values aren't supported (generated from unicode and keycode as fallback).

  4. OS on-screen keyboards emulate physical keyboard. 2 3

  5. Using custom virtual keyboard layout. 2

@Zireael07
Copy link
Contributor

Can that picture go into official Godot docs?

@bruvzg
Copy link
Member Author

bruvzg commented Dec 14, 2022

Can that picture go into official Godot docs?

In some form, I definitely plan to add it (#69751).

@bruvzg bruvzg force-pushed the key_unicode_actions branch from 4c8d2c0 to f6cf7bf Compare December 14, 2022 12:53
@bruvzg

This comment was marked as outdated.

@bruvzg bruvzg force-pushed the key_unicode_actions branch 4 times, most recently from 95e32a4 to 26ce1ab Compare December 21, 2022 09:51
@bruvzg bruvzg force-pushed the key_unicode_actions branch 6 times, most recently from 1810420 to b2f81a0 Compare December 28, 2022 07:51
@bruvzg bruvzg marked this pull request as ready for review December 28, 2022 07:51
@bruvzg bruvzg requested review from a team as code owners December 28, 2022 07:51
@bruvzg bruvzg force-pushed the key_unicode_actions branch from 501c9d1 to 331afa3 Compare January 23, 2023 13:04
@akien-mga akien-mga requested a review from raulsntos January 23, 2023 13:04
@bruvzg
Copy link
Member Author

bruvzg commented Jan 23, 2023

This might also be needed for NixOS which puts its includes in non-standard directories:

Added. I guess I should do #71263 for the X11 libs to avoid this kind of issues.

- Unify keycode values (secondary label printed on a key), remove unused hardcoded Latin-1 codes.
- Unify IME behaviour, add inline composition string display on Windows and X11.
- Add key_label (localized label printed on a key) value to the key events, and allow mapping actions to the unshifted Unicode events.
- Add support for physical keyboard (Bluetooth or Sidecar) handling on iOS.
- Add support for media key handling on macOS.

Co-authored-by: Raul Santos <raulsntos@gmail.com>
@bruvzg bruvzg force-pushed the key_unicode_actions branch from 331afa3 to daad4ae Compare January 23, 2023 13:08
@akien-mga
Copy link
Member

akien-mga commented Jan 23, 2023

Tested on Linux with en_US, fr_FR and da_DK keyboard layouts, seems to work great! Some tests with event.as_text_key_label():

French AZERTY, pressing all keys from top to bottom (remapping, on a physical US QWERTY):
²
Ampersand
É
QuoteDbl
Apostrophe
ParenLeft
Minus
È
UnderScore
Ç
À
ParenRight
Equal
A
Z
E
R
T
Y
U
I
O
P
BraceLeft
Dollar
Asterisk
Q
S
D
F
G
H
J
K
L
M
Ù
W
X
C
V
B
N
Comma
Semicolon
Colon
Exclam
The same on Danish keyboard layout:
½
1
2
3
4
5
6
7
8
9
0
Plus
Equal
Q
W
E
R
T
Y
U
I
O
P
Å
BraceRight
Apostrophe
A
S
D
F
G
H
J
K
L
Æ
Ø
Z
X
C
V
B
N
M
Comma
Period
Minus

I noticed a difference in the way modifiers are handled, but this may be intentional. Testing on French AZERTY, where:

  • Physical key "2" has key label "É", and "2" is the "shifted" key.
  • Physical key "E" has key label "E", and "€" is accessible via Alt Gr + E (on Linux with the default fr mapping).

Pressing Shift+É to get "2" prints:

Shift
Shift+É

Pressing Alt Gr+E to get "€" prints:

Alt
€

"€" is actually a key label on that layout: https://en.wikipedia.org/wiki/AZERTY

But the same also happens with extended mappings from the Linux keyboard layout which are not printed on the keys, e.g. Alt Gr+W gives "«" and Alt Gr+X gives "»". Those key combinations print:

Alt
«
Alt
»

It's probably different on Windows as these extended mappings are a Linux feature, so this can well be considered working as intended.

@akien-mga akien-mga merged commit 1f22c48 into godotengine:master Jan 23, 2023
@akien-mga
Copy link
Member

Thanks!

@vmedea
Copy link
Contributor

vmedea commented Jan 25, 2023

I'm sorry but I think the change in of SUPER_* and HYPER_* constants is a bit weird here.

From a X11/Wayland perspective the old situation makes more sense? Like, at least the left and right SUPER ("windows") key are really separate keys. I'm not sure about HYPER.

Also there's a stale SUPER_R/SUPER_L in core/keyboard.h, if you really intend to remove them.

@bruvzg
Copy link
Member Author

bruvzg commented Jan 25, 2023

I'm sorry but I think the change in of SUPER_* and HYPER_* constants is a bit weird here.

None of the other platforms distinguish L/R Super, all use META, and no other key have Left/Right variant. It is possible to get L/R variants on other platforms, but if it's done it should be done for all keys (Ctrl, Alt, etc.) not just for Super/Meta and essentially nonexistent Hyper. There was PR to do it, but it was rejected.

Also there's a stale SUPER_R/SUPER_L in core/keyboard.h, if you really intend to remove them.

Indeed, missed them, should be removed.

@vmedea
Copy link
Contributor

vmedea commented Jan 25, 2023

Ok, thanks, it wasn't clear to me that META was the same thing. I thought the key was entirely removed. Makes more sense then.

@Vivalzar
Copy link

I noticed a difference in the way modifiers are handled, but this may be intentional. Testing on French AZERTY, where:

  • Physical key "2" has key label "É", and "2" is the "shifted" key.

Yes, this is intentional. All numbers on French keyboards are accessed using shift.

@h0lley
Copy link

h0lley commented Feb 27, 2023

InputEventKey does not necessarily have the key_label set, such as when it has been retrieved from InputMap.action_get_events(). In that case, how do we obtain the localized key label of an unicode char that has no keycode representation (if it had, we could use DisplayServer.keyboard_get_keycode_from_physical()). Example: umlauts.

apologies if that is not within the scope of this PR. didn't find any other place related to key_label.

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