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

[Daniel Stone]

Hi,

On 8 March 2016 at 09:15, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> On Mon, 7 Mar 2016 19:25:54 +0100
> Hans de Goede <hdegoede at redhat.com> wrote:
>> On 07-03-16 19:23, Hans de Goede wrote:
>> > On 07-03-16 18:44, Olivier Fourdan wrote:
>> > 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.
>
> 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.

It was a deliberate choice. Compositors should always be responsive
and not being so is a pretty bad position to be in from the start;
clients have _even more_ information than the server about when repeat
is appropriate; sending them from the server to the client just causes
two sets of wakeups rather than one in order to perform repeat.
(Though, given they're likely to provoke rendering, that battle is
basically lost anyway.)

> 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,

Unspecified base, millisecond granularity, monotonic. Same as most X11
and Wayland timestamps really.

> though I suppose it'd be trivial to just
> maintain a timestamp and increment it by the repeat period. 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.
>
> 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?

We effectively already do this, so not a real problem tbh.

> 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.

This is my position as well.

> 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?

I'd imagine so, but if the situation is this bad anyway, clients just
queuing non-state-mutating changes (e.g. changed content buffer) may
swamp the compositor anyway. Unfortunately this seems to be basically
intractable until at least the point at which the Clutter (+ Cogl?)
and Mutter source trees are merged, so there's some urgency from that
corner to push an interim solution. Having looked at patch 2/2 though,
I don't think the same GTK+ solution is applicable to Xwayland, so
short of introducing server-side repeat into Wayland (a possibility
with a wl_keyboard version bump), I'm not sure what we can do here.

Cheers,
Daniel