[Spice-devel] [PATCH spice-gtk] RFC: widget: improve undesired key repeatition

[Hans de Goede]

Hi,

On 04/20/2012 01:59 AM, Marc-André Lureau wrote:
> Hi
>
> Now that I implemented it a bit more correctly, using a timer, and
> splitting the in two patches, I realize that it doesn't work with
> windows. Windows expect to receive several key press events
> apparently.

Hmm, that would mean that windows expect the keyboard itself to handle
keyrepeat. IOW it depends on hardware keyrepeat... ?

> Hans, the hack you mentionned in spicec doesn't seem to be
> what you said, since spicec client repeatly sends down messages when
> the key is pressed

Ah I was expecting spicec to have a key-state array like spice-gtk and
ignore the repeated press events since the key was already pressed. But
up on further examination of the code that is indeed not what is
happening, what is happening on key repeat is:

X-windows sends:
Keypress, timestamp X
Keyrelease + Keypress, both with timestamp X + something

spicec filters out the Keyrelease caused by X11 key-repeat, but it
does not filter out the press, so indeed it sends repeated down
messages, without any release/up messages in between (where as
X-windows does send release events in between).

> (btw, Gtk doesn't behave the way you described
> either, so this previous patch was working fine)

So gtk also filters out the release events and only gives repeated press
events for key-repeat?

> So it seems like we should keep sending repeat key press in fact. And
> it's probably better to rely on client-side repeat, since letting the
> server handling it may lead to precisely the bug I was trying to fix.
> And we should teach the agent to disable the key repeat function,
> which is a trival xset -r, or XGetKeyboardControl /
> XChangeKeyboardControl equivalent when a client is connected. Any
> other opinion?

I just tried the following:
1) start a linux guest
2) connect with either remote-viewer or spicec (does not matter)
3) start terminal, do xset -r
4) Keep the "a" key pressed

Result: no repeat

This is sort of what I was afraid of, the IMHO hack where the client
handles repeats and sends repeated pressed without releases in between
simply gets ignored on Linux guests, I tried with showkey and the kernel
does pass the press events but X11 simply ignores the repeated presses,
this also explains why we did not see much of a problem from this before,
otherwise we would have seen double the repeat rate under X11 before
(both from its internal repeat as from the extra events).

So we seem to have the following situation:

1) Windows wants / expect repeated press events for repeats, iow
repeat is handled client side, as we are currently doing

2) Linux ignores repeated press events, but has software keyrepeat
build into X11

Which also explains why we've only been seeing the spurious
key repeat with linux guests. Note I'm sure if all reports actually
are Linux guests, does someone know?

So we need to come up with some fix for 2. Options I see are:

1) Marc-André's initial approach, with the small delay, modified to
    keep the repeated keypress events, so as to not break windows.

2) Make Linux be more like windows, what I've in mind here is:
2.1) Add an agent-channel capability called "x11_client_side_keyrepeat"
2.2) Modify the linux agent and spice-gtk to support this capability
2.3) If both sides have the capability then:
2.4) The agent will do the equivalent of xset -r
2.5) spice-gtk will send release + press events on repeat instead
      of just press events

Disadvantage of 1:
-Adds extra latency to keypress handling, but we could make this
  configurable and disable it on low latency links / only enable
  it on high latency links

Disadvantage of 2:
-This causes the guest to think the user is hitting the key X times
  a second, some apps (ie spicec and spice-gtk) may show unexpecting
  behavior because of this

Regards,

Hans