Hi,
On 08-03-16 10:15, Pekka Paalanen wrote:
On Mon, 7 Mar 2016 19:25:54 +0100
Hans de Goede <[email protected]> wrote:
Hi,
On 07-03-16 19:23, Hans de Goede wrote:
Hi,
On 07-03-16 18:44, Olivier Fourdan wrote:
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.
Why not simply rely on the keyrepeat of the compositor and forward repeat
events from it ? That way xwayland will also end up using the compositors
keyrepeat settings which would mean the user does not need to worry
about configuring this in multiple places ?
Or is keyrepeat left to the toolkit under wayland ? In that case please
ignore my comment :)
OK, I've just seen that key-repeat is indeed left to the client / toolkit
under wayland. I'm wondering if that is a deliberate decision or more
of an accident, and if maybe we need a protocol ext for optional compositor
side key-repeat, that seems better then all clients needing to implement
this workaround.
Hi,
it might be an oversight or a desire for simplicity: wl_keyboard key
events are promised to be physically generated by input devices and
never faked by a compositor. Obviously compositor doing the key repeat
does not fit in that scheme. Key events also carry a timestamp of the
physical action gnerating it, and for compositors to fake events, it
would need to be able to fake the timestamp too. I don't know if
anything actually specifies what the clock for input events is, even
for evdev specifically, though I suppose it'd be trivial to just
maintain a timestamp and increment it by the repeat period.
Ack.
However,
that has the race that you might send a repeated event with a timestamp
greater than a following key release event which is sampled by the
kernel or hardware.
No, the whole idea of doing repeat in the server is that the server
can ensure that it has seen the release (e.g. by draining the evdev
fd for the device) before sending a repeat, so the whole idea is to avoid
the race.
You'd have to ask Kristian to be sure. Another question I have no idea
about is how you determine what keys repeat - would the compositor have
to maintain xkbcommon state for each client?
You could mark repeat events as such, actually they should probably
be named repeat-hint-events or something, and then the client can
decide whether it is a good idea to actually listen to the hint
and do key-repeat or not. So basically the idea would be to bring
the timing to the server side which has 2 advantages:
1) Server side this can be done race free
2) 1 common repeat-delay / speed setting for all apps / toolkits
And then let the client decide if it actually wants keyrepeat events
for the key in question or not, and if not simply drop them.
I understand having safeguards against the very busy-loop-flooding
issue that raised these patches is a huge improvement, but at the same
time IMHO there is a "bug" in the compositor that makes it unresponsive
or not fluent. This means the compositor is already having issues
providing a good user experience. That can of course be due to simply
too slow hardware, so something has to stop the flood.
Protecting XWayland against this flooding is probably necessary,
because XWayland is producing the repeat events which causes X11
clients to do things unthrottled. However, I'm not sure the *same* fix
should be applied to native Wayland clients.
Wouldn't native Wayland clients have means to throttle their state
changes to compositor response already? E.g. switching to/from
fullscreen you have to wait for a configure event, then you draw in new
state, and then you wait for a frame callback to have the new state
show on screen. Only after all that, it makes sense to change state
again. Does this not solve the issue for native Wayland apps?
This specific issue yet, but what about scrolling down by keeping
the cursor down key pressed releasing it when you are where you want
to be and then scrolling down some more because of latency in the
release event processing and then the client doing some more repeat
events because it has no idea of this latency ?
Note another solution would be some sort of hartbeat ping from
the server with a high enough frequency for clients to use as a timer
and clients using this as timer source for keyrepeat. This is more or
less what the current patch suggested by Olivier for Xwayland is doing.
I'd imagine that scheme to also improve user experience if any of the
compositor or the app are stalling: a user presses F11, nothing happens
(yet). User presses it again. Then the first press gets executed,
followed by the second press. Is the user left with a changed or the
original state? Wouldn't the user prefer the changed state, since the
original state is the only thing he has seen so far, and he did press
the key to change it?
Regards,
Hans
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
