wayland_common: handle pressed keys in keyboard_enter event

[na-na-hi]

This is required by the wayland protocol to keep the logical keyboard state consistent. Quoting https://gitlab.freedesktop.org/wayland/wayland/-/commit/9e4f25692792df679660b390324842c19c0031a5:

In the wl_keyboard logical state, this event sets the active surface to the surface argument and the keys currently logically down to the keys in the keys argument.

[github-actions[bot]]

Download the artifacts for this pull request:

Windows

* [mpv-i686-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1527324638.zip) * [mpv-x86_64-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1527333066.zip) * [mpv-x86_64-windows-msvc](https://nightly.link/mpv-player/mpv/actions/artifacts/1527358535.zip)

macOS

* [mpv-macos-12-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1527321098.zip) * [mpv-macos-13-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1527323032.zip) * [mpv-macos-14-arm](https://nightly.link/mpv-player/mpv/actions/artifacts/1527320048.zip)

[mahkoh]

This is incorrect. mpv must not synthesize key presses from the keys in the enter event. This is impossible for multiple reasons:

• mpv cannot know which modifiers are active at the time of the enter event
• mpv cannot know the order in which keys were pressed.

[na-na-hi]

mpv must not synthesize key presses from the keys in the enter event.

It does not. MP_KEY_STATE_DOWN and MP_KEY_STATE_UP merely set the key press/release state in the mpv input system, which has the final say on the effect the keys have.

• mpv cannot know which modifiers are active at the time of the enter event

Incorrect. The compositor can simply send the modifiers event before sending the enter event. This is explicitly allowed by the protocol: The compositor may send this event without a surface of the client having keyboard focus

• mpv cannot know the order in which keys were pressed.

This is underspecified in the protocol, but the order can be conveyed by specifying how the order of the keys in keys in the enter event corresponds to the order in which keys were pressed.

[mahkoh]

It does not. MP_KEY_STATE_DOWN and MP_KEY_STATE_UP merely set the key press/release state in the mpv input system, which has the final say on the effect the keys have.

I don't know anything about the mpv input system but when I

1. focus mpv
2. press (and hold) M to mute
3. focus another window
4. focus mpv

Then mpv will unmute itself again at 4 even though I only pressed M once. So evidently some input was synthesized.

[na-na-hi]

Then mpv will unmute itself again at 4 even though I only pressed M once.

The behavior is the same as on X11. This also matches what I expect from platforms which have server-side autorepeat: if I hold down a key while the window is unfocused and then focus the window afterwards, the autorepeat kicks in and the program responds to the key events (e.g. starts inputting text for a text editor).

[mahkoh]

The behavior is the same as on X11.

Maybe so but that is not how any other wayland applications behave. If I hold X while the application is not focused and then click on a text field inside the application, the application will not start inserting the letter X in the text field until I release and press X again.

I've verified this explicitly with

• alacritty
• gtk
• qt
• firefox
• chromium

right now and I don't remember seeing different behavior in any other applications except mpv on current master.

[mahkoh]

Also I don't think auto repeat has anything to do with this since holding M in mpv does not toggle between mute and unmute at the auto repeat rate.

[na-na-hi]

Maybe so but that is not how any other wayland applications behave. If I hold X while the application is not focused and then click on a text field inside the application, the application will not start inserting the letter X in the text field until I release and press X again.

On both Windows and X11, the applications do start inserting the letter X in this case. All programs you mentioned don't behave the same on Wayland, and I consider it a bug in these programs/toolkits if the aim is to have consistent autorepeat behavior across all platforms, because as this PR demonstrates it is possible to handle this in the Wayland protocol.

since holding M in mpv does not toggle between mute and unmute at the auto repeat rate

This is a deliberate decision by mpv. Some commands are by default repeatable (e.g. seeking), others aren't. This can be controlled with the repeatable input command prefix, so it's possible to do so if you want it to toggle at the auto repeat rate.

[mahkoh]

All programs you mentioned don't behave the same on Wayland, and I consider it a bug in these programs/toolkits if the aim is to have consistent autorepeat behavior across all platforms

Saying that all of these applications and toolkits, that are largely developed independently, are all buggy in exactly the same way, even though they have supported wayland for many years, is an exceptional claim that requires exceptional evidence. For example, links to bug reports in their bug trackers showing that they consider this a bug instead of intentional behavior.

because as this PR demonstrates it is possible to handle this in the Wayland protocol.

This PR does not demonstrate this for the reasons I mentioned above. You are making significant assumptions about compositor behavior that is not guaranteed

The compositor can simply send the modifiers event before sending the enter event.

The protocol does not guarantee that the compositor does this. The protocol only requires the compositor to send a modifiers event after the enter event:

The compositor must send the wl_keyboard.modifiers event after this event.

the order can be conveyed by specifying how the order of the keys in keys in the enter event corresponds to the order in which keys were pressed

The protocol does not require the compositor to do this. The compositor is free to store the pressed keys in a hashmap which makes the order random.

---

In any case, you linked a commit above saying that the protocol requires the behavior from this PR. I'm the author of this commit and I can tell you that that was not my intention nor the intention of any of the other people involved in it. Our intention was only to restrict what events compositors are allowed to send in which situations.

[na-na-hi]

This PR does not demonstrate this for the reasons I mentioned above. You are making significant assumptions about compositor behavior that is not guaranteed

If this is true, then this PR exposed a flaw in the Wayland protocol which means it cannot guarantee consistent autorepeat behavior as on other platforms. The Wayland protocol should be amended to address this.

The protocol does not guarantee that the compositor does this.

As said above, this is a flaw in the Wayland protocol because it's underspecified. In this case, as with the norm with Wayland protocols, it's the compositor's policy whether this is handled or not. If a compositor doesn't do this, then it cannot enable clients handle autorepeat the same as on other platforms, and this is OK, since the client can do nothing about it, the compositor takes the blame, like with the situation with server-side decoration.

The compositor is free to store the pressed keys in a hashmap which makes the order random.

Ultimately it does not matter: conceptually in the Wayland keyboard logical state, all keys are pressed at the same time in the prespective of the client, so random order is fine.

Our intention was only to restrict what events compositors are allowed to send in which situations.

Then please amend the protocol to clarify that clients are not meant to use this information for tracking keyboard state, and suggest an alternative method to handle the mentioned autorepeat problem.

[mahkoh]

If this is true, then this PR exposed a flaw in the Wayland protocol which means it cannot guarantee consistent autorepeat behavior as on other platforms. The Wayland protocol should be amended to address this.

It is not a goal of wayland to behave like other windowing systems.

suggest an alternative method to handle the mentioned autorepeat problem.

There was no problem to begin with. Before this PR, mpv behaved as expected from a wayland application.

[na-na-hi]

It is not a goal of wayland to behave like other windowing systems.

I'm afraid that you're not in a position to say this for the whole Wayland ecosystem. It's up to individual compositor's policy to determine the correct behavior in the constraints specified by the Wayland protocol. Some of them are formalized as new protocols which fix various deficiencies which other windowing systems don't have.

Before this PR, mpv behaved as expected from a wayland application.

There is nothing can be "expected from a wayland application" since AFAIK Wayland doesn't have an official HIG nor have any other official stance on this. On the other hand, if the keys argument isn't meant to be used by the client, why does it exist in the first place?

Other applications don't handle it not because it's more correct to do so, but because they think it's unimportant, or are likely never aware of this situation to begin with, since it's handled by the server autorepeat on other platforms, no special handling is needed, but it requires explicit handling on Wayland.

[Dudemanguy]

Yeah sorry I don't think "other wayland applications don't do this" is really an argument when the compositor sends us an event with keys that are logically down according to the protocol. If you really don't want clients to do this, please amend the wayland protocol so compositors don't send this event with keys.

I would say most applications are simply not aware of this situation so they don't try to handle it. I personally was not until this PR.

[mahkoh]

If you really don't want clients to do this, please amend the wayland protocol so compositors don't send this event with keys.

If the intention was that clients treat the keys in the enter event as key presses, then the keys in the enter event would not be necessary. The compositor could simply send key press events immediately after the enter event. The keys in the enter event exist precisely so that clients can distinguish between "the keys that are pressed" and "key press events".

There are clients that are interested in the keys that are pressed, such as overlays that want to display a virtual keyboard. But mpv is not one of them. Mpv should only be interested in discrete press and release events.

[na-na-hi]

The compositor could simply send key press events immediately after the enter event.

This violates the Wayland protocol according to your own words:

The compositor must not send this event if state is pressed (resp. released) and the key was already logically down (resp. was not logically down) immediately before this event.

[mahkoh]

It doesn't if the compositor left the keys argument in the enter event empty as @Dudemanguy suggested.

[na-na-hi]

True, but this would be a protocol version change since keys is currently documented to contain "the currently pressed keys". Also there needs to be a policy on how to handle keys pressed in enter and key events differently for this change to be meaningful.

[mahkoh]

There is no ambiguity. Everyone has been able to figure it out as my list of applications above shows.

[na-na-hi]

There is no ambiguity. Everyone has been able to figure it out as my list of applications above shows.

Your list of applications will eventually behave the same as mpv currently does if the purposed protocol change requiring keys to be empty is adopted.