rust-windowing / winit

Window handling library in pure Rust
https://docs.rs/winit/
Apache License 2.0
4.74k stars 891 forks source link

Remove synthetic key events? #3543

Open madsmtm opened 6 months ago

madsmtm commented 6 months ago

Raised in https://github.com/rust-windowing/winit/pull/3538#discussion_r1508670322.

Should we remove synthetic keyboard events (those that contain is_synthetic)?

What are the benefits and the drawbacks?

kchibisov commented 6 months ago

I'd rather move them out of normal events, because folks handle them but these events shouldn't trigger general input.

dhardy commented 6 months ago

Synthetic press events: I don't have a use for them.

Synthetic release events (i.e. matching a real press): these are useful to notify that a press-and-hold sequence should be terminated.

kchibisov commented 6 months ago

Yeah, but you generally should assume that every key is lifted upon focus lost, since that's mostly the only case where it can happen iirc. But yeah, releases make more sense than presses, since people may think that they should input from synthetic presses, while in really it's not the case.

dhardy commented 6 months ago

Sure, I could handle the key-releases on my end.

I guess the only real thing is whether keys held while the window does not have focus are handled correctly.

I can trick my text editor like this, but it's unimportant: focus another window, press and hold Alt, click to focus the editor, press F. The File menu is opened but the access keys are not underlined like usual.

More important might be something like Blender where modifier keys affect what click+drag does. But to handle that correctly the synthetic key press has to arrive before the mouse-click event, and that's still not good enough to handle cursors (in case the modifier key should change the mouse cursor seen on hover).

It is also possible to handle this stuff without synthetic key presses; they just make that easier (assuming they do the right thing).

kchibisov commented 6 months ago

You shouldn't handle represses when you had something latched upon focus though, the general recommendation to never do so, because it's always not intended. That's the general advice by core wayland protocol as well and the same with x11, I think.

I can trick my text editor like this, but it's unimportant: focus another window, press and hold Alt, click to focus the editor, press F. The File menu is opened but the access keys are not underlined like usual.

You generally shouldn't replay button presses, because user could use this modifier as a part of e.g. Alt+Tab so you end up releasing and showing the menu on focus from such switcher, this is most of the time not desired from the UX point of view, because user haven't asked for that.

It is also possible to handle this stuff without synthetic key presses; they just make that easier (assuming they do the right thing).

ModifiersChanged is not synthetic and always broadcasted to you and rebroadcasted on focus change, so if you want e.g. shift+mouse drag it should just work, with the exception of unfocused clients where the modifiers are considered empty now, though it'll likely change. Keep in mind that some Wayland compositors don't send modifiers when the application is not focused even when you have mouse pointer above it, though wayland-core allows that. We have #2768 for that and it'll be addressed.

hut commented 4 months ago

For reference, here is an incomplete list of UI frameworks that mistakenly interpret synthetic key presses as real key presses:

I'm guessing every UI framework will make this mistake, until, after years of subtle customer confusion/annoyance, they realize what is going on and fix the bug on their end.

Perhaps it would be best to hide the synthetic key press events by default.

alice-i-cecile commented 3 months ago

Can these be emitted as a different event, rather than variants of a keyboard event? That would allow them to be passed through unchanged, but avoid the current footgun.

kchibisov commented 3 months ago

That what I was saying, we should just route them separately, it's just at the current state of things we can not do much about them for 0.30.

We could still add a warning in docs, but IIRC the presence of such events was desired before https://github.com/rust-windowing/winit/issues/1272.

dhardy commented 3 months ago

Is a simpler solution possible: never emit synthetic key-press events but do emit synthetic key-release events?

Or make synthetic events opt-in?

NEON725 commented 2 months ago

Can these be emitted as a different event, rather than variants of a keyboard event? That would allow them to be passed through unchanged, but avoid the current footgun.

This solution has my vote. Also reduces the boiler plate in the typical case, without limiting functionality for any devs that have a legitimate use case for interpreting these events.