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

Tracking issue: improve Key input #3653

Open
emilk opened this issue Nov 28, 2023 · 17 comments
Open

Tracking issue: improve Key input #3653

emilk opened this issue Nov 28, 2023 · 17 comments
Labels
eframe Relates to epi and eframe egui good first issue Good for newcomers help wanted Extra attention is needed Tracking issue

Comments

@emilk
Copy link
Owner

emilk commented Nov 28, 2023

The keyboard input in egui is limited in a couple of ways:

  • Lots of keys missing from Event::Key
  • We don't report the super/meta modifier key
  • We don't report modifier key presses (just their state)
  • Physical keys not reported on the eframe web backend
  • KeyboardShortcut should ignore shift and alt keys for logical keys (e.g. Ctrl Plus may require pressing shift on some keyboards). See The keyboard shortcut of zoom in does not work on the JIS keyboard #3626 for more

How can I help?

  • Look at the code for egui::Key and follow the instructions there for adding new keys
  • Investigate if we can get physical keycode on web

Requested keys

Prioir art

Relevant issues

Physical vs Logical keys

Consider someone owning a physical QWERTY keyboard:
qwerty

However, they have remapped it to use Dvorak (perhaps even repainting the text on their keyboard):
dvorak

If they start hitting Caps Lock and then moving to the left, the keys they will be hitting will be

  • physical (qwerty): CapsLock, A, S, D, F, …
  • logical (Dvorak): CapsLock, A, O, E, U …

The logical keys makes sense for most keyboard shortcuts - if you say "Press Cmd+S to save" in your UI, the user will be looking for the logical key "S"
The physical keys are mostly useful for games, where e.g. te physical WSAD on QWERTY moves a character, no matter what the users keymap is.

@emilk emilk added eframe Relates to epi and eframe egui labels Nov 28, 2023
@emilk emilk changed the title Distinguish physical and logical keys Tracking issue: improve Key input Nov 28, 2023
@emilk emilk added this to the 0.25.0 milestone Nov 28, 2023
@emilk emilk added help wanted Extra attention is needed good first issue Good for newcomers labels Nov 28, 2023
@rustbasic
Copy link
Contributor

rustbasic commented Nov 29, 2023

First, store the input: winit::event::KeyboardInput from egui-winit on_keyboard_input().

Then, how about providing the input value to someone who wants detailed keyboard input information in pub struct InputState, pub struct RawInput, or pub struct ViewportInfo?

@emilk
Copy link
Owner Author

emilk commented Dec 27, 2023

@rustbasic egui is backend agnostic, and must not depend on winit

@emilk emilk mentioned this issue Dec 28, 2023
@samitbasu
Copy link
Contributor

We use egui extensively at work, and the lack of support for the numpad has been noted (and complained about). I will take a look at this issue and see if I can help out, unless anyone else is already doing it.

CC: @emilk @twitzelbos

@samitbasu
Copy link
Contributor

@emilk - Just to clarify... Splitting Key into a logical and physical key will necessitate breaking the integration API, correct? Since e.g., Key will become a struct with 2 fields instead of remaining an enum? Or did you envision having a separate event that sends physical keys from one that sends logical keys?

@emilk
Copy link
Owner Author

emilk commented Jan 9, 2024

I'm not 100% convinced we should split the current enum Key. It is very convenient to just have one enum, and physical keys are also a pretty niche use case.

This was referenced Jan 14, 2024
emilk added a commit that referenced this issue Jan 15, 2024
* Part of #3653 

Also move `enum Key` to own file
@emilk emilk removed this from the Next Major Release milestone Jan 29, 2024
@HactarCE
Copy link
Contributor

HactarCE commented Feb 21, 2024

Physical and logical keys are fundamentally different things, and in every successful system that handles both they are treated separately. They just happen to align most of the time on QWERTY. I'm working on an application where many users want to bind numpad keys separately from the number row.

I would love to see egui use keyboard_types, as it looks very complete.

For an example of converting winit events to a shared representation, see my key-names crate. You can try it online in Hyperspeedcube (Help → Keybinds reference to see keys in real-time, or Settings → Puzzle keybinds and click on one of the keys to see it decode scan codes). key-names also detects the user's keyboard layout on Windows and Linux, although it's not possible on web or macOS. I don't expect this from egui but it's just so that I can show the keybinds reference with the correct labels.

@TicClick
Copy link
Contributor

for visibility, #4081 also belongs here

@KarKarbI4
Copy link

@emilk @TicClick

The logical keys makes sense for most keyboard shortcuts - if you say "Press Cmd+S to save" in your UI, the user will be looking for the logical key "S"
The physical keys are mostly useful for games, where e.g. te physical WSAD on QWERTY moves a character, no matter what the users keymap is.

The thing is non-latin keyboards have both latin and non-latin symbols printed. And when user wants to, for example, save document, they presses CTRL+ physycal "S" key, which logical key could be is some non-latin symbol.

I've checked web demo, input shortcuts (CTRL+A, CTRL+Z) don't work when using Greek layout.

To fix the issue, I would consider using physycal keys fallback for shortcuts processing for non-latin layouts as described here

Speaking of issue Physical keys not reported on the eframe web backend, I can make a PR, which maps web_sys::KeyboardEvent.code() to physycal_key for web backend.

After that PR is merged, I can create PR, which add physycal_key fallback for shortcuts processing for non-latin layouts

@parasyte
Copy link
Contributor

The thing is non-latin keyboards have both latin and non-latin symbols printed. And when user wants to, for example, save document, they presses CTRL+ physycal "S" key, which logical key could be is some non-latin symbol.

It isn't quite the physical "S" key that is used for "Ctrl+S", though. It's the virtual "S", which happens to commonly map to the physical location for "S" on Western keyboards. And therefore, also shares the same scancode. Have a look at the Bulgarian keyboard layout, for instance: https://kbdlayout.info/kbdbulg; VK_S maps to "U+044F CYRILLIC SMALL LETTER YA". And its scancode is 1F (Just like the US layout: https://kbdlayout.info/kbdus/scancodes).

You might question, then, "what's the difference?" The difference is that VK_S is not always positioned on the left-side... For instance, here's one of the Turkish layout variants: https://kbdlayout.info/kbdtuf/virtualkeys; VK_S is positioned on the lower right! VK_OEM_4 sits in the position where you would expect to find "S". And if you check the virtual key map, sure enough, the physical scancode 1F maps to VK_OEM_4, also as we would expect.

The clear thing to do is exactly what was proposed:

  • Actions that want the virtual representation of a letter or character (like "Ctrl+S") should use the virtual key.
  • Actions that want the physical position of the key (like a game with WASD movement on a US Qwerty layout, or ",AOE" on US Dvorak layouts) should use the scancode.

The scancode is very unlikely to be what you want for the former case. The linked SO answer is wrong, as I have demonstrated with just a few samples.

@myandrienko
Copy link

myandrienko commented Feb 24, 2024

@parasyte Turkish F is a latin layout though, so the logical key will be used, and Ctrl + S will still work, even though the scancode is 32 and not 1F. The problem only arises if the user has a keyboard with both Turkish F and non-latin layout (like ЙЦУКЕН), and presses Ctrl + S while in ЙЦУКЕН layout. But dual layouts like this are so rare, they are almost not a thing in practice.

As a dual layout user, I think @KarKarbI4's suggestion makes a lot of sense. An overwhelming majority of non-latin keyboard users have a keyboard with QWERTY + their non-latin layout printed. This will provide a good enough support for them, instead of no support at all which is the current state.

@parasyte
Copy link
Contributor

parasyte commented Feb 25, 2024

Changing the layout may change the virtual keys reported. It is unclear why there is any confusion on this.

To add to this comment: any user may opt for the Turkish F layout, regardless of what is printed on their key caps. That is inconsequential to the layout.

@myandrienko
Copy link

Changing the layout may change the virtual keys reported. It is unclear why there is any confusion on this.

@parasyte There's no confusion about this. My comment is related to the proposed solution:

Actions that want the virtual representation of a letter or character (like "Ctrl+S") should use the virtual key.

In general, I agree with it. But the proposed solution doesn't take into consideration the users of non-latin keyboard layouts. For example, the user of the Bulgarian keyboard layout will not be able to invoke the "Ctrl+S" action.

Now, most users of the Bulgarian layout will also have US QWERTY or a similar layout set up. However, it's bad user experience to switch to another layout to invoke a keyboard shortcut. So I propose to make the following addition to your solution:

Actions that want the virtual representation of a letter or character (like "Ctrl+S") should use the virtual key. When a non-latin virtual letter is received, the action should be invoked as if the virtual letter corresponding to the same scancode in QWERTY layout was pressed.

I think that addition makes sense because on most localized keyboards QWERTY layout is printed on keycaps alongside the local layout, so users usually expect that keyboard shortcuts work in the way I described.

I hope the proposal makes more sense now. What do you think?

@parasyte
Copy link
Contributor

For example, the user of the Bulgarian keyboard layout will not be able to invoke the "Ctrl+S" action.

There are two things going on. On the one hand, an example of a keyboard shortcut was provided for illustration without regard to i18n or l10n. And on the other hand, we are discussing keyboard events, which is only tangentially related to i18n and l10n (and accessibility in general).

We can’t really discuss i18n, l10n, and a11y without also bringing up IME. And I think you’ll agree it is a large topic without a great deal of overlap with handling keyboard layouts.

I would probably make an adjustment to the example keyboard shortcut that included common “save” commands in other languages to disambiguate what is really being proposed.

But I am adamant that scancodes (aka physical positions) are never what is wanted for normal human interactions outside of niche cases like games.

@myandrienko
Copy link

myandrienko commented Feb 25, 2024

@parasyte Hmm, I have a feeling we have a misunderstanding here. The topic of keyboard shortcuts has very little to do with i18n. You're saying:

I would probably make an adjustment to the example keyboard shortcut that included common “save” commands in other languages to disambiguate what is really being proposed.

That's not how keyboard shortcuts usually work. No matter what system or interface language you have enabled, the "save" command is always "Ctrl+S". I use two keyboard layouts (QWERTY and ЙЦУКЕН), and I probably switch them a thousand times a day. It would drive me crazy if all the keyboard shortcuts would change depending on my current layout.

So what do you do if there's no letter S in your local keyboard layout? Well, your keyboard probably has QWERTY layout engraved on it along with the local one - so just find the letter S there and press it.

image

On this keyboard with ЙЦУКЕН layout you will still press "Ctrl+S" to save. The virtual key in this case will be Ы, and the "save" action will not fire as expected. Not good.

Take a look at this screenshot: even though Chrome UI is in Russian, keyboard shortcut hints are still referring to the latin letters, for this exact reason:

image

That does not mean however that we should just rely on scancodes. Like you said, in most cases that's not what normal humans want :) If I'm using Dvorak, or Turkish F, or any other non-QWERTY layout with latin letters, I still want to press the button with the letter S on my keyboard and for it to invoke an action for "Ctrl+S".

So, to reiterate my previous comment: when a non-latin letter is pressed, we should find the corresponding letter in QWERTY layout and invoke an action for it.

@parasyte
Copy link
Contributor

I see that my interpretation of keyboard shortcuts in various languages is far from perfect. If it is always the case that "Ctrl+S" is for saving, then there is little reason to internationalize it. But there is a misconception that the "S" is in relation to QWERTY layout.

This is precisely why the Windows virtual key system uses, e.g. VK_S. When identified by virtual key, the key is simply called VK_S. Even if the text that will be entered is Ы, it's still VK_S for purposes of the virtual key map. The W3C's UI Events specification (and by extension winit, which is based on UI Events) is a weird mixture attempting to unify platform differences for virtual keys and text entry, and largely pays no mind to scancodes.

I think that the discussion is off the rails because the UI Events semantics have been implied on one side and platform-specific virtual key mapping on the other. Virtual key maps don't deal with Unicode at all, there isn't a Ы virtual key! That's a concern of text entry (which encompasses IME, composition keys, etc.)

@parasyte
Copy link
Contributor

@rustbasic I'm not sure it would be wise to just jump right in, considering how much confusion there already is with existing terminology. The differences are subtle between physical (0x1F), virtual (VK_S, kVK_ANSI_S, ...), and logical (s, S, ы, Ы, ς, Σ, and any other character that can be input with this one weird key), but it's important to understand the role that each level of abstraction plays.

@psethwick
Copy link
Contributor

I have a need for the super key
(specifically I need to filter out the key combo I use to start my app .... egui starts too fast)

my first thought was this:
/// On Windows, the Windows key. On Linux, the super key (often.. the Windows key)
/// On Mac will always be false
pub super_: bool,
on Modifiers

but now I'm wondering if there's a reason not to rename mac_cmd to super_ send it on all platforms?

emilk pushed a commit that referenced this issue May 10, 2024
…ve (#4461)

resolves #4081 (see discussion
starting from
#3653 (comment) for
extra context)

this partly restores event-emitting behaviour to the state before #3649,
when shortcuts such as `Ctrl` + `C` used to work regardless of the
active layout. the difference is that physical keys are only used in
case of the logical ones' absence now among the named keys.

while originally I have only limited this to clipboard shortcuts
(Ctrl+C/V/X), honestly it felt like a half-assed solution. as a result,
I decided to expand this behaviour to all key events to stick to the
original logic, in case there are other workflows and hotkeys people
rely on or expect to work out of the box. let me know if this is an
issue.
emilk added a commit that referenced this issue Jun 23, 2024
emilk added a commit that referenced this issue Jun 23, 2024
* Part of #3653

<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md)
before opening a Pull Request!

* Keep your PR:s small and focused.
* The PR title is what ends up in the changelog, so make it descriptive!
* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->

It was surprising to me that this key wasn't sending `Event::Key` events
(only `Event::Text`), so I added a `Key` for it.

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
hacknus pushed a commit to hacknus/egui that referenced this issue Oct 30, 2024
…ve (emilk#4461)

resolves emilk#4081 (see discussion
starting from
emilk#3653 (comment) for
extra context)

this partly restores event-emitting behaviour to the state before emilk#3649,
when shortcuts such as `Ctrl` + `C` used to work regardless of the
active layout. the difference is that physical keys are only used in
case of the logical ones' absence now among the named keys.

while originally I have only limited this to clipboard shortcuts
(Ctrl+C/V/X), honestly it felt like a half-assed solution. as a result,
I decided to expand this behaviour to all key events to stick to the
original logic, in case there are other workflows and hotkeys people
rely on or expect to work out of the box. let me know if this is an
issue.
hacknus pushed a commit to hacknus/egui that referenced this issue Oct 30, 2024
hacknus pushed a commit to hacknus/egui that referenced this issue Oct 30, 2024
* Part of emilk#3653

<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md)
before opening a Pull Request!

* Keep your PR:s small and focused.
* The PR title is what ends up in the changelog, so make it descriptive!
* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->

It was surprising to me that this key wasn't sending `Event::Key` events
(only `Event::Text`), so I added a `Key` for it.

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
eframe Relates to epi and eframe egui good first issue Good for newcomers help wanted Extra attention is needed Tracking issue
Projects
None yet
Development

No branches or pull requests

9 participants