[PATCH xserver 0/2] RFC: Sync key repeat with Wayland compositor

[Benoit Gschwind]

Hello,

Here is my little contribution to the discussion, following a discussion
on #wayland irc channel.

The issue discussed here, as far I understood, is due to a client that
miss interpret a high latency of the compositor as a repeating key. This
happen when a client receive a key press event but do not receive the
corresponding release event for a large enough time even if this event
have physically happened.

The current xwayland is an instance of unexpected consequence of this
issue but this issue is not limited to xwayland. Thus this issue must be
fixed at the protocol level and not at software level.

Trying to take in account all gathered comments, I built the following
proposal.

First I propose to keep current behavior, i.e. client receive press and
release events. But when a client is about to guess a repeating event,
instead of guessing it, it have to request the server to provide
repeating events. To do so, the client send a request with a parameter
that represent a frequency or a special value (like 0) and something to
identify the related key press event. if a key release have happened
before the repeat request the server send the key release immediately
and ignore the repeat request, else he send a repeat event immediately
and try to send repeat event at the requested frequency until key
release happen or another repeating request arrive. if a new repeating
request arrive the frequency is updated to the new one if necessary or
the repeat is canceled if the frequency is set to a special value (let's
say 0). if the client does not receive release event or repeating key,
he have to guess that the server is busy and wait for events.

This approach is very flexible, allowing client to request repeating
only when needed. This solve the issue above and ensure that client get
valid repeat event. This approach also avoid that the client make a
request on every guessed repeat event.

Best regards.









Le 07/03/2016 18:44, Olivier Fourdan a écrit :
> Key repeat is handled by the X server, but for Wayland, the key
> press/release events need to be processed and forwarded by the Wayland
> compositor first.
> 
> If the Wayland compositor get stuck for whatever reason and does not
> process the key release event fast enough, this may cause the Xwayland
> server to generate spurious key repeat events.
> 
> That actually can lead to a complete hang of both the Wayland compositor
> and Xwayland as seen in the following GNOME bug:
> 
>   https://bugzilla.gnome.org/show_bug.cgi?id=762618
> 
> Basically, what happens here is that the Wayland compositor takes longer
> to actually process the fullscreen/unfullscreen transition than the
> repeat delay.
> 
> As a result, while the user has released the F11 key that triggers the 
> fullscreen/unfullscreen transition, the Wayland compositor is stuck
> is a graphical operation and cannot process the key release events, which
> triggers the auto-repeat in Xwayland, and therefore cause even more 
> fullscreen transitions. Only way out here is to kill the Xwayland
> application and wait for the queued up event to clear out...
> 
> While this is arguably a bug in the Wayland compositor, there is no
> way that I can think of to guarantee that this cannot happen.
> 
> One possiblity to mitigate the problem is to block the key repeat until
> we are sure the Wayland compositor is actually processing events.
> 
> The idea comes from a similar patch for gtk+ by Ray Strode here:
> 
>   https://bug757942.bugzilla-attachments.gnome.org/attachment.cgi?id=322876
> 
> from bug https://bugzilla.gnome.org/show_bug.cgi?id=757942
> 
> While the following patches do fix the issue in my case, they are not
> perfect and only a mitigation of the problem (e.g. they could easily be
> defeated by a Wayland compositor that would have different priority for
> Wayland protocol and input processing), that's why I send these couple
> of patches as an RFC for now.
> 
> Cheers,
> Olivier
> 
> 
> Olivier Fourdan (2):
>   xkb: add hook to allow/deny AccessX key repeat
>   xwayland: add a server sync before repeating keys
> 
>  hw/xwayland/xwayland-input.c | 36 ++++++++++++++++++++++++++++++++++++
>  include/xkbsrv.h             |  6 ++++++
>  xkb/xkbAccessX.c             |  4 +++-
>  3 files changed, 45 insertions(+), 1 deletion(-)
>