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

wl_keyboard::enter has key state information that Winit ignores #3909

Closed
1 task done
sodiboo opened this issue Sep 11, 2024 · 3 comments
Closed
1 task done

wl_keyboard::enter has key state information that Winit ignores #3909

sodiboo opened this issue Sep 11, 2024 · 3 comments
Labels
B - bug Dang, that shouldn't have happened DS - wayland

Comments

@sodiboo
Copy link

sodiboo commented Sep 11, 2024

Description

The state of a wl_keyboard object generally has this cycle, to my understanding:

  • The keyboard (i.e. the wl_keyboard object) is unfocused and you cannot know what is pressed. It is undefined, and you should not assume anything. In practice, applications just keep their last state.
  • The keyboard gets focus, and with it we get an array of all the pressed keys. See wl_keyboard::enter
  • The keyboard receives key events whenever the logical state of a key is changed (i.e. pressed or released)
  • The keyboard is eventually unfocused. We do not get any more updates on the key states after that point.

This repeats. When the keyboard gains focus (enter), we get the full state immediately, and we get any further updates. The enter event has a field called keys, which is an array of the currently pressed keys.

Winit doesn't handle the enter event properly, its keys field is not even bound to any name, and thus dropped immediately:

WlKeyboardEvent::Enter { surface, .. } => {

This, in general, means that winit consumers don't get accurate key state events unless that key has been pressed after the last enter event. Furthermore (and this isn't a winit issue), some clients then handle this incorrectly, assuming different semantics to the key events sent by winit. This has been plaguing me for months in the Smithay winit backend, and i only just now traced it back to winit. See Smithay/smithay#1353 (comment). Smithay's way to handle it is basically just wrong for the wl_keyboard interface, but it is worth noting that if winit were to keep track of pressed keys, and synthesize key press/release events upon the enter event for the keys whose state changed, then Smithay's current code would actually work as-is.

Debugging output

No response

Window isn't shown unless you draw

  • I understand that windows aren't shown on Wayland unless I draw and present to them.

Winit version

  • Smithay was using winit 0.29.2 when i first noticed this issue and completely misdiagnosed it.
  • Smithay currently uses winit 0.30.0, so that's the version i've reproduced the issue in today before tracing it back to winit.
  • The code i reference is present in the latest commit to the master branch.
@sodiboo sodiboo added B - bug Dang, that shouldn't have happened DS - wayland labels Sep 11, 2024
@kchibisov
Copy link
Member

This, in general, means that winit consumers don't get accurate key state events unless that key has been pressed after the last enter event. Furthermore (and this isn't a winit issue), some clients then handle this incorrectly, assuming different semantics to the key events sent by winit.

Clients generally shouldn't handle any of that, since those key presses, are not meant to be handled, unless you do forwarding which 99% of clients don't.

Smithay's way to handle it is basically just wrong for the wl_keyboard interface, but it is worth noting that if winit were to keep track of pressed keys, and synthesize key press/release events upon the enter event for the keys whose state changed, then Smithay's current code would actually work as-is.

smithay should use modifiers event, which we send correctly, yes it won't be perfect right now, but it'll solve the issue as I told you already on the niri issue. It's really not that hard.

Duplicate of #1272.

@sodiboo
Copy link
Author

sodiboo commented Sep 11, 2024

Duplicate of #1272.

I see. I looked for other issues mentioning the fact that the keys field of the enter event is ignored, but found none when looking like that.

@kchibisov
Copy link
Member

This state will be forwarded eventually, once we have a different way to do this, other than synthetic event like it's now, but for now I don't in the mood in changing that because you generally should never process synthetic events unless you really know what you're doing.

And sending synthetic releases is wrong because some people assume that those are real releases, while you should never react to them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B - bug Dang, that shouldn't have happened DS - wayland
Development

No branches or pull requests

2 participants