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

Integration with winit #19

Open
1 of 9 tasks
madsmtm opened this issue Jan 31, 2023 · 10 comments
Open
1 of 9 tasks

Integration with winit #19

madsmtm opened this issue Jan 31, 2023 · 10 comments

Comments

@madsmtm
Copy link

madsmtm commented Jan 31, 2023

Greetings @pyfisch!

I see that you've previously helped out a lot in pushing winit forwards on better keyboard handling (though this was before my own time as maintainer though, so we haven't really interacted, and I'm quite out of the loop on it, do say so if I'm mistaken about something!)

We've recently come closer to finishing that age-old work, see rust-windowing/winit#2662 (comment) - after that, it would be nice to try to consolidate winit's needs for keyboard types with this crate's types.

I see that you've proposed merging things before in rust-windowing/winit#753 (comment), and have created the branch winit to do some of this work already, is this still something you would be interested in helping out with?

Doing a quick diff between the implementations reveal:

  • Missing F25-F35 variants on Code and Key
  • We might want to support letting the user know if the left vs. right modifier key is being held; is this something you be open to support on Modifiers at some point? (e.g. LALT/RALT, LCTRL/RCTRL, ...)
  • winit has a bit of a different serialization logic for Modifiers
  • We enrich the Unidentified key code to allow access to the raw platform-specific scancode.
  • Meta vs. Super is confusing, could perhaps be merged into a single thing?
  • Key::Character(String) is harder to match on than e.g. &str, and may be more inefficient than e.g. Cow<'static, str>.
    • Unsure about this one, I know @maroider has some thoughts on this, and that has also been discussed at length
  • winit has Key::Dead(Option<char>) to allow specify the character which is inserted when pressing the dead-key twice
  • Key::Space is missing, though perhaps Key::Character(" ") is indeed cleaner
  • We try to follow a MSRV-policy of "works with ~7 months-old Rust versions", would you be willing to try to adhere to that as well (as a minimum)?

A proposed solution there was adding an extension parameter to KeyboardEvent, Key and Code like this:

enum Code<Extra = !> {
    // ...
    Extra(Extra),
}

enum Key<Extra = !> {
    // ...
    Extra(Extra),
}

struct KeyboardEvent<Extra = ()> {
    // ...
    pub extra: Extra,
}

Along with a method like without_extra that discards any extra data.

That would allow us to keep the ergonomics higher than matching on e.g. WinitCode { code: Code::Backquote, .. }.


Relatedly, it would also be nice to use your CompositionEvent, for this we may need to specify a cursor along with that - so next time you're doing a breaking release, might want to consider #[non_exhaustive] on that too - but let's focus on Key, Code and KeyboardEvent.

@madsmtm
Copy link
Author

madsmtm commented Jan 31, 2023

Tracking issue on winit's side for this: rust-windowing/winit#2394

@pyfisch
Copy link
Owner

pyfisch commented Feb 4, 2023

Greetings @madsmtm

Great to hear, that the keyboard is making progress and close to the finish line.

Missing F25-F35 variants on Code and Key

Sure I can add them.

We might want to support letting the user know if the left vs. right modifier key is being held; is this something you be open to support on Modifiers at some point? (e.g. LALT/RALT, LCTRL/RCTRL, ...)

The modifiers are a shortcut to test if keys with certain functions are pressed. I don't think it is very common to check if a specifically LALT vs RALT is pressed, when this is needed an app can directly listen to keydown events. However I am open to adding these in addition to the existing modifiers.

winit has a bit of a different serialization logic for Modifiers

I would need to check what the exact differences are.

* [ ]  We enrich the `Unidentified` key code to allow access to the raw platform-specific scancode.

I'm considering to make Key generic: Key<Scancode=()>, would this help?

* [ ]  `Meta` vs. `Super` is confusing, could perhaps be merged into a single thing?

Super considered to be a legacy key by https://w3c.github.io/uievents-key/#keys-modifier so you probably don't need to use it in winit. I can't remove it from this crate however because it's part of the aforementioned standard.

* [ ]  `Key::Character(String)` is harder to `match` on than e.g. `&str`, and may be more inefficient than e.g. `Cow<'static, str>`. 
  * Unsure about this one, I know @maroider has some thoughts on this, and that has also been discussed at length

Maybe this can be resolved by making Key::Character generic?
Cows also hard to match, right?

* [ ]  `winit` has `Key::Dead(Option<char>)` to allow specify the character which is inserted when pressing the dead-key twice

I think this is better solved with CompositionEvent (or a variant specific to winit).

* [ ]  `Key::Space` is missing, though perhaps `Key::Character(" ")` is indeed cleaner

https://w3c.github.io/uievents-key/#keys-unicode (Example 2)
All space characters are stored as "Key::Character", why special-case one? ;-)

* [ ]  We try to follow a MSRV-policy of "works with ~7 months-old Rust versions", would you be willing to try to adhere to that as well (as a minimum)?

That should be fine. keyboard-types changes rarely and doesn't need the newest features. However I haven't checked the policies of the dependencies (bitflags, serde, unicode-segmentation).

A proposed solution there was adding an extension parameter to KeyboardEvent, Key and Code like this:

enum Code<Extra = !> {
    // ...
    Extra(Extra),
}

enum Key<Extra = !> {
    // ...
    Extra(Extra),
}

struct KeyboardEvent<Extra = ()> {
    // ...
    pub extra: Extra,
}

Along with a method like without_extra that discards any extra data.

That would allow us to keep the ergonomics higher than matching on e.g. WinitCode { code: Code::Backquote, .. }.

When would you use Extra for Key and Code? I haven't tested this but it may be fairly easy to add new key values if you can demonstrate that they are available on existing keyboards.

Relatedly, it would also be nice to use your CompositionEvent, for this we may need to specify a cursor along with that - so next time you're doing a breaking release, might want to consider #[non_exhaustive] on that too - but let's focus on Key, Code and KeyboardEvent.

I'll keep this in mind.

@eugenesvk
Copy link

The modifiers are a shortcut to test if keys with certain functions are pressed. I don't think it is very common to check if a specifically LALT vs RALT is pressed

That's a common pain point for me in most of the apps since I use Right⎇› as a "symbol" key as I never use it for regular alt-y functions, so when apps don't differentiate between sides, I can't use it anymore :(

when this is needed an app can directly listen to keydown events

maybe I'm missing something, but what if the events happen outside of an app? E.g., I press Right⇧› to do something in another app and then mouse-switch to the oroginal app, the original app won't get the Right⇧› event, it'd just receive a generic is down when another key is pressed?
Also, it's probably a bit harder if you need to listen directly rather than checking for an existing enum variant?

@pyfisch
Copy link
Owner

pyfisch commented Aug 12, 2023

That's a common pain point for me in most of the apps since I use Right⎇› as a "symbol" key as I never use it for regular alt-y functions, so when apps don't differentiate between sides, I can't use it anymore :(

I would look into configuring your keyboard layout so that the key that is usually used for Right-Alt is mapped to the function you use it for. Then apps should not recognize the key as an Alt key.
I am aware that some apps use the physical position of keys instead of the logical function, however this needs to be fixed in apps and keyboard-types already does the right thing by keeping these concepts separate.

E.g., I press Right⇧› to do something in another app and then mouse-switch to the oroginal app, the original app won't get the Right⇧› event, it'd just receive a generic ⇧ is down when another key is pressed?

Yes, that is correct.

@eugenesvk
Copy link

configuring your keyboard layout so that the key that is usually used for Right-Alt is mapped to the function you use it for

Not sure how that's remappable, the keyboard layout I set, e.g., on a Mac via Ukelele with all the symbol dead keys can't change modifier keys, and the issue is that the system is also not smart enough to differentiate ‹left vs right› modifiers at this level, but then this level of direct lyout configuration is the only one that allows system keyboard viewer hints and activation in text fields since it's just like regular alpha keys
So I can't turn right alt into some kind of "hyper" modifier

Advanced remapper apps like Karabiner are smarter in the left/right sense, but then you lose some of the things that depend on the actual layout

And both of of these systems are blind to the app state (e.g., they don't know whether you're in an insert or command mode in a text field), so in the end the only proper way is for the app to handle input correclty, and that's there where an app would need all the help it can get from crates like yours :)

@pyfisch
Copy link
Owner

pyfisch commented Aug 12, 2023

@eugenesvk I am not familiar with Mac OS, so I don't know how keyboard layouts work there. However from your description the remapping functionality seems quite limited. I don't think effort needed to implement this feature on all platforms (it's most likely impossible on the web) is worth it. I won't add modifiers for left/right location.

@eugenesvk
Copy link

Why is it not worth it? Near-space keys like Alt/Cmd are the most ergonomic modifiers since you can use thumb fingers instead of pinky-Shifts, and currently this potential is wasted even in the newly designed Rust ecosystem (the web can remain bugged alone)

@pyfisch
Copy link
Owner

pyfisch commented Aug 12, 2023

With this crate I primarily want to provide access to a simple keyboard API modeled on the web API. In my opinion remapping keys is a task best handled by the operating system, it's a bummer that Mac OS doesn't support the functions you want.

@eugenesvk
Copy link

With this crate I primarily want to provide access to a simple keyboard API modeled on the web API.

Ok, just from your previous comment I assumed that bug-for-bug compatibility isn't a requirement

However I am open to adding these in addition to the existing modifiers.

In my opinion remapping keys is a task best handled by the operating system,

But those OS don't exist, doing it properly would mean the OS should be aware of all the apps' internal logic/state, which is only possible if apps signal that state

it's a bummer that Mac OS doesn't support the functions you want.

None of the OS do, but this issue is not tied to the OS, it's about apps built on top of the modifier/key lists in your library, which doesn't support side differentiation of modifiers and requires more complicated yet still faulty logic

Even if your suggestion with keyboard layout were possible (let's assume you can turn ⎇› into some hyper key that would act as a real modifier (current workaround is to make this into a 4-key combo)), it would still not solve the issue properly as even then the user would have to rebind all the +X key combos inside the app since in some states he'd want ⎇›X to issue a command, and in some other app states he'd want to use the layout function and insert a symbol

The only proper solution is for the app to be aware of the full info re. a key combo, and this includes physical modifiers, not just logical modifiers, same for non-modifier keys which do signal their location

@pyfisch
Copy link
Owner

pyfisch commented Aug 12, 2023

I misunderstood your use-case and didn't pick up on the fact that you want to use the key for different functions depending on context. However this discussion is going nowhere as I am not planning to change this crates API for this.

@pyfisch pyfisch mentioned this issue Aug 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants