[Qemu-devel] [PATCH] vnc: add additional key up event before repeated key down

[Qemu-devel] [PATCH] vnc: add additional key up event before repeated key down

[Chunyan Liu]

Using xen tools 'xl vncviewer' with tigervnc (default on SLE-12),
found that: the display of the guest is unexpected while keep
pressing a key. We expect the same character multiple times, but
it prints only one time. This happens on a PV guest in text mode.

After debugging, found that tigervnc sends repeated key down events
in this case, to differentiate from user pressing the same key many
times. Vnc server only prints the character when it finally receives
key up event.

To solve this issue, this patch tries to add additional key up event
before the next repeated key down event (if the key is not a control
key).

Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 ui/vnc.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/ui/vnc.c b/ui/vnc.c
index f8d9b7d..a265378 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1659,6 +1659,25 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
         if (down)
             vs->modifiers_state[keycode] ^= 1;
         break;
+    default:
+        if (qemu_console_is_graphic(NULL)) {
+            /* record key 'down' info. Some client like tigervnc
+             * will send key down repeatedly if user pressing a
+             * a key for long time. In this case, we should add
+             * additional key up event before repeated key down,
+             * so that it can display the key multiple times.
+             */
+            if (down) {
+                if (vs->modifiers_state[keycode]) {
+                    /* add a key up event */
+                    do_key_event(vs, 0, keycode, sym);
+                }
+                vs->modifiers_state[keycode] = 1;
+            } else {
+                vs->modifiers_state[keycode] = 0;
+            }
+        }
+        break;
     }
 
     /* Turn off the lock state sync logic if the client support the led
-- 
1.8.4.5

Re: [Qemu-devel] [PATCH] vnc: add additional key up event before repeated key down

[Stefano Stabellini]

On Fri, 5 Sep 2014, Chunyan Liu wrote:
> Using xen tools 'xl vncviewer' with tigervnc (default on SLE-12),
> found that: the display of the guest is unexpected while keep
> pressing a key. We expect the same character multiple times, but
> it prints only one time. This happens on a PV guest in text mode.
> 
> After debugging, found that tigervnc sends repeated key down events
> in this case, to differentiate from user pressing the same key many
> times. Vnc server only prints the character when it finally receives
> key up event.

Is this actually how a vnc client should behave?
How do the vnc client and server from realvnc behave in this regard
(they are the reference implementation)?


> To solve this issue, this patch tries to add additional key up event
> before the next repeated key down event (if the key is not a control
> key).
> 
> Signed-off-by: Chunyan Liu <cyliu@suse.com>
> ---
>  ui/vnc.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index f8d9b7d..a265378 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -1659,6 +1659,25 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
>          if (down)
>              vs->modifiers_state[keycode] ^= 1;
>          break;
> +    default:
> +        if (qemu_console_is_graphic(NULL)) {
> +            /* record key 'down' info. Some client like tigervnc
> +             * will send key down repeatedly if user pressing a
> +             * a key for long time. In this case, we should add
> +             * additional key up event before repeated key down,
> +             * so that it can display the key multiple times.
> +             */
> +            if (down) {
> +                if (vs->modifiers_state[keycode]) {
> +                    /* add a key up event */
> +                    do_key_event(vs, 0, keycode, sym);
> +                }
> +                vs->modifiers_state[keycode] = 1;
> +            } else {
> +                vs->modifiers_state[keycode] = 0;
> +            }

I am not 100% convinced that we have to fix this in QEMU, but if we do
then this doesn't look like the right fix: the switch statement that you
are modifying is for QEMU console switches. Also if I am not mistaken
modifiers_state is meant for control keys, not regular keys.

At the very least you could move the change below, near the call to
qemu_input_event_send_key_number.


> +        }
> +        break;
>      }
>  
>      /* Turn off the lock state sync logic if the client support the led
> -- 
> 1.8.4.5
> 

Re: [Qemu-devel] [PATCH] vnc: add additional key up event before repeated key down

[Chun Yan Liu]

>>> On 9/6/2014 at 05:23 AM, in message
<alpine.DEB.2.02.1409052202451.2334@kaball.uk.xensource.com>, Stefano
Stabellini <stefano.stabellini@eu.citrix.com> wrote: 
> On Fri, 5 Sep 2014, Chunyan Liu wrote: 
> > Using xen tools 'xl vncviewer' with tigervnc (default on SLE-12), 
> > found that: the display of the guest is unexpected while keep 
> > pressing a key. We expect the same character multiple times, but 
> > it prints only one time. This happens on a PV guest in text mode. 
> >  
> > After debugging, found that tigervnc sends repeated key down events 
> > in this case, to differentiate from user pressing the same key many 
> > times. Vnc server only prints the character when it finally receives 
> > key up event. 
>  
> Is this actually how a vnc client should behave? 
> How do the vnc client and server from realvnc behave in this regard 
> (they are the reference implementation)? 

VNC protocol doesn't specify how to handle key repetition. Tightvnc
sends key-down&key-up repeatedly, but some example like RealVNC for
Windows does the same thing - it sends only repeated key-down.

Generally the VNC keyboard handling gives lot of space for interpretation
and so the implementations differ.

>  
>  
> > To solve this issue, this patch tries to add additional key up event 
> > before the next repeated key down event (if the key is not a control 
> > key). 
> >  
> > Signed-off-by: Chunyan Liu <cyliu@suse.com> 
> > --- 
> >  ui/vnc.c | 19 +++++++++++++++++++ 
> >  1 file changed, 19 insertions(+) 
> >  
> > diff --git a/ui/vnc.c b/ui/vnc.c 
> > index f8d9b7d..a265378 100644 
> > --- a/ui/vnc.c 
> > +++ b/ui/vnc.c 
> > @@ -1659,6 +1659,25 @@ static void do_key_event(VncState *vs, int down, int  
> keycode, int sym) 
> >          if (down) 
> >              vs->modifiers_state[keycode] ^= 1; 
> >          break; 
> > +    default: 
> > +        if (qemu_console_is_graphic(NULL)) { 
> > +            /* record key 'down' info. Some client like tigervnc 
> > +             * will send key down repeatedly if user pressing a 
> > +             * a key for long time. In this case, we should add 
> > +             * additional key up event before repeated key down, 
> > +             * so that it can display the key multiple times. 
> > +             */ 
> > +            if (down) { 
> > +                if (vs->modifiers_state[keycode]) { 
> > +                    /* add a key up event */ 
> > +                    do_key_event(vs, 0, keycode, sym); 
> > +                } 
> > +                vs->modifiers_state[keycode] = 1; 
> > +            } else { 
> > +                vs->modifiers_state[keycode] = 0; 
> > +            } 
>  
> I am not 100% convinced that we have to fix this in QEMU, but if we do 
> then this doesn't look like the right fix: the switch statement that you 
> are modifying is for QEMU console switches.
> Also if I am not mistaken 
> modifiers_state is meant for control keys, not regular keys. 

Yes, you are right. Currently modifiers_state is used for control keys, and
key-down info is not recorded. I just want to reuse modifiers_state to
record the key-down info. Since control key and regular key have different
keycodes, that won't affect control key handling.

>  
> At the very least you could move the change below, near the call to 
> qemu_input_event_send_key_number. 

Could change there too, but need to check the keycode again: only regular key
needs the change, control key should not be changed. So I just move the change
ahead in the switch.

>  
>  
> > +        } 
> > +        break; 
> >      } 
> >   
> >      /* Turn off the lock state sync logic if the client support the led 
> > --  
> > 1.8.4.5 
> >  
>  
>  

Re: [Qemu-devel] [PATCH] vnc: add additional key up event before repeated key down

[Markus Armbruster]

"Chun Yan Liu" <cyliu@suse.com> writes:

>>>> On 9/6/2014 at 05:23 AM, in message
> <alpine.DEB.2.02.1409052202451.2334@kaball.uk.xensource.com>, Stefano
> Stabellini <stefano.stabellini@eu.citrix.com> wrote: 
>> On Fri, 5 Sep 2014, Chunyan Liu wrote: 
>> > Using xen tools 'xl vncviewer' with tigervnc (default on SLE-12), 
>> > found that: the display of the guest is unexpected while keep 
>> > pressing a key. We expect the same character multiple times, but 
>> > it prints only one time. This happens on a PV guest in text mode. 
>> >  
>> > After debugging, found that tigervnc sends repeated key down events 
>> > in this case, to differentiate from user pressing the same key many 
>> > times. Vnc server only prints the character when it finally receives 
>> > key up event. 
>>  
>> Is this actually how a vnc client should behave? 
>> How do the vnc client and server from realvnc behave in this regard 
>> (they are the reference implementation)? 
>
> VNC protocol doesn't specify how to handle key repetition. Tightvnc
> sends key-down&key-up repeatedly, but some example like RealVNC for
> Windows does the same thing - it sends only repeated key-down.
>
> Generally the VNC keyboard handling gives lot of space for interpretation
> and so the implementations differ.

If implementations differ, and QEMU already behaves like some of them,
then why change it?  What exactly gets fixed and what gets broken by the
proposed change?

[...]

Re: [Qemu-devel] [PATCH] vnc: add additional key up event before repeated key down

[Chun Yan Liu]

>>> On 9/10/2014 at 02:23 AM, in message <87tx4girg6.fsf@blackfin.pond.sub.org>,
Markus Armbruster <armbru@redhat.com> wrote: 
> "Chun Yan Liu" <cyliu@suse.com> writes: 
>  
>>>>> On 9/6/2014 at 05:23 AM, in message 
> > <alpine.DEB.2.02.1409052202451.2334@kaball.uk.xensource.com>, Stefano 
> > Stabellini <stefano.stabellini@eu.citrix.com> wrote:  
> >> On Fri, 5 Sep 2014, Chunyan Liu wrote:  
> >> > Using xen tools 'xl vncviewer' with tigervnc (default on SLE-12),  
> >> > found that: the display of the guest is unexpected while keep  
> >> > pressing a key. We expect the same character multiple times, but  
> >> > it prints only one time. This happens on a PV guest in text mode.  
> >> >   
> >> > After debugging, found that tigervnc sends repeated key down events  
> >> > in this case, to differentiate from user pressing the same key many  
> >> > times. Vnc server only prints the character when it finally receives  
> >> > key up event.  
> >>   
> >> Is this actually how a vnc client should behave?  
> >> How do the vnc client and server from realvnc behave in this regard  
> >> (they are the reference implementation)?  
> > 
> > VNC protocol doesn't specify how to handle key repetition. Tightvnc 
> > sends key-down&key-up repeatedly, but some example like RealVNC for 
> > Windows does the same thing - it sends only repeated key-down. 
> > 
> > Generally the VNC keyboard handling gives lot of space for interpretation 
> > and so the implementations differ. 
>  
> If implementations differ, and QEMU already behaves like some of them, 
> then why change it?

To change qemu side because we could not expect each VNC client behaves
the same when holding key down, some sending key-down, key-up, key-down,
key-up; but some sending key-down, key-down, key-down .... Without change,
client only sending key-down, key-up, key-down,key-up ... can get correct
display.

>  What exactly gets fixed and what gets broken by the 
> proposed change?

Holding the key down, only one character is printed, but repeated characters
are expected. Happens on some vnc client. Either vnc client or vnc server
should change some to match.
 
>  
> [...] 
>  
>  

Re: [Qemu-devel] [PATCH] vnc: add additional key up event before repeated key down

[Anthony Liguori]

On Tue, Sep 9, 2014 at 8:31 PM, Chun Yan Liu <cyliu@suse.com> wrote:
>
>
>>>> On 9/10/2014 at 02:23 AM, in message <87tx4girg6.fsf@blackfin.pond.sub.org>,
> Markus Armbruster <armbru@redhat.com> wrote:
>> "Chun Yan Liu" <cyliu@suse.com> writes:
>>
>>>>>> On 9/6/2014 at 05:23 AM, in message
>> > <alpine.DEB.2.02.1409052202451.2334@kaball.uk.xensource.com>, Stefano
>> > Stabellini <stefano.stabellini@eu.citrix.com> wrote:
>> >> On Fri, 5 Sep 2014, Chunyan Liu wrote:
>> >> > Using xen tools 'xl vncviewer' with tigervnc (default on SLE-12),
>> >> > found that: the display of the guest is unexpected while keep
>> >> > pressing a key. We expect the same character multiple times, but
>> >> > it prints only one time. This happens on a PV guest in text mode.
>> >> >
>> >> > After debugging, found that tigervnc sends repeated key down events
>> >> > in this case, to differentiate from user pressing the same key many
>> >> > times. Vnc server only prints the character when it finally receives
>> >> > key up event.
>> >>
>> >> Is this actually how a vnc client should behave?
>> >> How do the vnc client and server from realvnc behave in this regard
>> >> (they are the reference implementation)?
>> >
>> > VNC protocol doesn't specify how to handle key repetition. Tightvnc
>> > sends key-down&key-up repeatedly, but some example like RealVNC for
>> > Windows does the same thing - it sends only repeated key-down.
>> >
>> > Generally the VNC keyboard handling gives lot of space for interpretation
>> > and so the implementations differ.
>>
>> If implementations differ, and QEMU already behaves like some of them,
>> then why change it?
>
> To change qemu side because we could not expect each VNC client behaves
> the same when holding key down, some sending key-down, key-up, key-down,
> key-up; but some sending key-down, key-down, key-down .... Without change,
> client only sending key-down, key-up, key-down,key-up ... can get correct
> display.

The VNC keyboard handling is pretty straight forward.   The keys sent
are symbolic and correspond to input events (as interpreted by an
application).  Whether you get repeat events depends on a lot of
client side configuration.

>>  What exactly gets fixed and what gets broken by the
>> proposed change?
>
> Holding the key down, only one character is printed, but repeated characters
> are expected. Happens on some vnc client. Either vnc client or vnc server
> should change some to match.

You should fix TigerVNC.  It's broken if it isn't sending repeat events.

Regards,

Anthony Liguori

>>
>> [...]
>>
>>
>
>
>

Re: [Qemu-devel] [PATCH] vnc: add additional key up event before repeated key down

[Chun Yan Liu]

>>> On 9/10/2014 at 11:47 AM, in message
<CA+aC4ktktHx8VtvKSvSoH-YZtF_EE0-U2DO=y55wRUZjzSf8gA@mail.gmail.com>, Anthony
Liguori <anthony@codemonkey.ws> wrote: 
> On Tue, Sep 9, 2014 at 8:31 PM, Chun Yan Liu <cyliu@suse.com> wrote: 
> > 
> > 
> >>>> On 9/10/2014 at 02:23 AM, in message <87tx4girg6.fsf@blackfin.pond.sub.org>, 
> > Markus Armbruster <armbru@redhat.com> wrote: 
> >> "Chun Yan Liu" <cyliu@suse.com> writes: 
> >> 
> >>>>>> On 9/6/2014 at 05:23 AM, in message 
> >> > <alpine.DEB.2.02.1409052202451.2334@kaball.uk.xensource.com>, Stefano 
> >> > Stabellini <stefano.stabellini@eu.citrix.com> wrote: 
> >> >> On Fri, 5 Sep 2014, Chunyan Liu wrote: 
> >> >> > Using xen tools 'xl vncviewer' with tigervnc (default on SLE-12), 
> >> >> > found that: the display of the guest is unexpected while keep 
> >> >> > pressing a key. We expect the same character multiple times, but 
> >> >> > it prints only one time. This happens on a PV guest in text mode. 
> >> >> > 
> >> >> > After debugging, found that tigervnc sends repeated key down events 
> >> >> > in this case, to differentiate from user pressing the same key many 
> >> >> > times. Vnc server only prints the character when it finally receives 
> >> >> > key up event. 
> >> >> 
> >> >> Is this actually how a vnc client should behave? 
> >> >> How do the vnc client and server from realvnc behave in this regard 
> >> >> (they are the reference implementation)? 
> >> > 
> >> > VNC protocol doesn't specify how to handle key repetition. Tightvnc 
> >> > sends key-down&key-up repeatedly, but some example like RealVNC for 
> >> > Windows does the same thing - it sends only repeated key-down. 
> >> > 
> >> > Generally the VNC keyboard handling gives lot of space for interpretation 
> >> > and so the implementations differ. 
> >> 
> >> If implementations differ, and QEMU already behaves like some of them, 
> >> then why change it? 
> > 
> > To change qemu side because we could not expect each VNC client behaves 
> > the same when holding key down, some sending key-down, key-up, key-down, 
> > key-up; but some sending key-down, key-down, key-down .... Without change, 
> > client only sending key-down, key-up, key-down,key-up ... can get correct 
> > display. 
>  
> The VNC keyboard handling is pretty straight forward.   The keys sent 
> are symbolic and correspond to input events (as interpreted by an 
> application).  Whether you get repeat events depends on a lot of 
> client side configuration. 
>  
> >>  What exactly gets fixed and what gets broken by the 
> >> proposed change? 
> > 
> > Holding the key down, only one character is printed, but repeated  
> characters 
> > are expected. Happens on some vnc client. Either vnc client or vnc server 
> > should change some to match. 
>  
> You should fix TigerVNC.  It's broken if it isn't sending repeat events.

About this question, my colleague sent mail to TigerVNC upstream before,
but they are not convinced  that TigerVNC behaves wrong. VNC protocol
doesn't specify which way is right for pressing key for long time.
(key-down, key-up, key-down, key-up...
or key-down, key-down, key-down, ...., key-down, key-up)
To them, they don't think 'key-down, key-down, ..., key-down, key-up' is
wrong. RealVNC for Windows also send the same events as TigerVNC.

Yeah, it's really a disputable thing. SPICE is much better in this case.
 
>  
> Regards, 
>  
> Anthony Liguori 
>  
> >> 
> >> [...] 
> >> 
> >> 
> > 
> > 
> > 
>  
>  

Re: [Qemu-devel] [PATCH] vnc: add additional key up event before repeated key down

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> On Tue, Sep 9, 2014 at 8:31 PM, Chun Yan Liu <cyliu@suse.com> wrote:
>>
>>
>>>>> On 9/10/2014 at 02:23 AM, in message
>>>>> <87tx4girg6.fsf@blackfin.pond.sub.org>,
>> Markus Armbruster <armbru@redhat.com> wrote:
>>> "Chun Yan Liu" <cyliu@suse.com> writes:
>>>
>>>>>>> On 9/6/2014 at 05:23 AM, in message
>>> > <alpine.DEB.2.02.1409052202451.2334@kaball.uk.xensource.com>, Stefano
>>> > Stabellini <stefano.stabellini@eu.citrix.com> wrote:
>>> >> On Fri, 5 Sep 2014, Chunyan Liu wrote:
>>> >> > Using xen tools 'xl vncviewer' with tigervnc (default on SLE-12),
>>> >> > found that: the display of the guest is unexpected while keep
>>> >> > pressing a key. We expect the same character multiple times, but
>>> >> > it prints only one time. This happens on a PV guest in text mode.
>>> >> >
>>> >> > After debugging, found that tigervnc sends repeated key down events
>>> >> > in this case, to differentiate from user pressing the same key many
>>> >> > times. Vnc server only prints the character when it finally receives
>>> >> > key up event.
>>> >>
>>> >> Is this actually how a vnc client should behave?
>>> >> How do the vnc client and server from realvnc behave in this regard
>>> >> (they are the reference implementation)?
>>> >
>>> > VNC protocol doesn't specify how to handle key repetition. Tightvnc
>>> > sends key-down&key-up repeatedly, but some example like RealVNC for
>>> > Windows does the same thing - it sends only repeated key-down.
>>> >
>>> > Generally the VNC keyboard handling gives lot of space for interpretation
>>> > and so the implementations differ.
>>>
>>> If implementations differ, and QEMU already behaves like some of them,
>>> then why change it?
>>
>> To change qemu side because we could not expect each VNC client behaves
>> the same when holding key down, some sending key-down, key-up, key-down,
>> key-up; but some sending key-down, key-down, key-down .... Without change,
>> client only sending key-down, key-up, key-down,key-up ... can get correct
>> display.
>
> The VNC keyboard handling is pretty straight forward.   The keys sent
> are symbolic and correspond to input events (as interpreted by an
> application).  Whether you get repeat events depends on a lot of
> client side configuration.
>
>>>  What exactly gets fixed and what gets broken by the
>>> proposed change?
>>
>> Holding the key down, only one character is printed, but repeated characters
>> are expected. Happens on some vnc client. Either vnc client or vnc server
>> should change some to match.
>
> You should fix TigerVNC.  It's broken if it isn't sending repeat events.

It *is* sending repeat events.  The commit message says so, and I
tested it to confirm.

Key repeat works just fine for me in the guest, both in Grub and in
Linux.  It obviously doesn't for Chunyan Liu "on a PV guest in text
mode."  I suspect that PV guest's handling of key repeat is what needs
fixing.

Re: [Qemu-devel] [PATCH] vnc: add additional key up event before repeated key down

[Peter Maydell]

On 17 September 2014 00:04, Markus Armbruster <armbru@redhat.com> wrote:
> Anthony Liguori <anthony@codemonkey.ws> writes:
>> You should fix TigerVNC.  It's broken if it isn't sending repeat events.
>
> It *is* sending repeat events.  The commit message says so, and I
> tested it to confirm.

The question of course is what "repeat events" actually means,
both to the VNC protocol and for the semantics of QEMU's internal
input API functions. It looks like the original RBD protocol
docs fail to state how key repeat should be handled.
Googling found somebody's improved version of the protocol docs
https://github.com/sibson/vncdotool/blob/master/docs/rfbproto.rst#keyevent
which says that key repeat should be handled by the client sending
"down down down down down down up" (which is what TigerVNC is
reported to do here). Obviously any clients which take the other
choice and send "down up down up down up" will generally seem
to work pretty much OK because the only distinction is if the guest
looks at the precise difference between keys being held down or
not. So I think our VNC server implementation should also follow
the "down down down up" interpretation of the spec.

Which brings us to the other half of this: what does our
UI layer specify should be the behaviour for key repeat?
Gerd, can you clarify what the common input layer's expectation
is here? Should UI front ends call qemu_input_event_send_key()
with 'down/down/down/up' or 'down/up/down/up' semantics?
(Obviously if you're a UI front end then you're going to have
to convert if your host OS's windowing system has the opposite
set of semantics or some other way of indicating held-down-repeat
like X11's "if the timestamp on "up" and "down" is identical
this is key-repeat. And keyboard-hardware device models
may need to convert again if the semantics of the h/w they're
modelling don't match the common input layer.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] vnc: add additional key up event before repeated key down

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 17 September 2014 00:04, Markus Armbruster <armbru@redhat.com> wrote:
>> Anthony Liguori <anthony@codemonkey.ws> writes:
>>> You should fix TigerVNC.  It's broken if it isn't sending repeat events.
>>
>> It *is* sending repeat events.  The commit message says so, and I
>> tested it to confirm.
>
> The question of course is what "repeat events" actually means,
> both to the VNC protocol and for the semantics of QEMU's internal
> input API functions. It looks like the original RBD protocol
> docs fail to state how key repeat should be handled.
> Googling found somebody's improved version of the protocol docs
> https://github.com/sibson/vncdotool/blob/master/docs/rfbproto.rst#keyevent
> which says that key repeat should be handled by the client sending
> "down down down down down down up" (which is what TigerVNC is
> reported to do here). Obviously any clients which take the other
> choice and send "down up down up down up" will generally seem
> to work pretty much OK because the only distinction is if the guest
> looks at the precise difference between keys being held down or
> not. So I think our VNC server implementation should also follow
> the "down down down up" interpretation of the spec.

I agree.

One way to fill the gap in the spec is "the protocol does not allow for
repeating keys, so we need to do the next best thing and pretend the
user hit the key repeatedly".  This dumbs down a repeating keypress to
repeated keypresses.

Another way is "everybody knows how repeating keys work (down, down,
..., down, up), and that's why the the protocol doesn't mention it".

If the sending side dumbs down repeating keypress to repeated
keypresses, the receiving side cannot distinguish one from the other
anymore.  Bad.

If the sending side sends repeating keys, but the receiving side doesn't
understand that, key repeat doesn't work.  Also bad.

Making the receiving side understand has no downside, unlike making the
sending side dumb things down.

> Which brings us to the other half of this: what does our
> UI layer specify should be the behaviour for key repeat?
> Gerd, can you clarify what the common input layer's expectation
> is here? Should UI front ends call qemu_input_event_send_key()
> with 'down/down/down/up' or 'down/up/down/up' semantics?
> (Obviously if you're a UI front end then you're going to have
> to convert if your host OS's windowing system has the opposite
> set of semantics or some other way of indicating held-down-repeat
> like X11's "if the timestamp on "up" and "down" is identical
> this is key-repeat. And keyboard-hardware device models
> may need to convert again if the semantics of the h/w they're
> modelling don't match the common input layer.)

PC keyboards send "down, down, ..., down, up".  A quick glance at the
code suggests our device model expects to get exactly that from the
input layer.

Re: [Qemu-devel] [PATCH] vnc: add additional key up event before repeated key down

[Gerd Hoffmann]

  Hi,

> Which brings us to the other half of this: what does our
> UI layer specify should be the behaviour for key repeat?
> Gerd, can you clarify what the common input layer's expectation
> is here? Should UI front ends call qemu_input_event_send_key()
> with 'down/down/down/up' or 'down/up/down/up' semantics?

It isn't formally specified anywhere.  The UIs usually simply pass
through the key events they get.  IMO it is more useful to go for
down/down/down/up.  This is how PS/2 works, and this allows the guest to
figure whenever it's autorepeat or really multiple key presses.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] vnc: add additional key up event before repeated key down

[Peter Maydell]

On 17 September 2014 04:24, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> Which brings us to the other half of this: what does our
>> UI layer specify should be the behaviour for key repeat?
>> Gerd, can you clarify what the common input layer's expectation
>> is here? Should UI front ends call qemu_input_event_send_key()
>> with 'down/down/down/up' or 'down/up/down/up' semantics?
>
> It isn't formally specified anywhere.  The UIs usually simply pass
> through the key events they get.  IMO it is more useful to go for
> down/down/down/up.  This is how PS/2 works, and this allows the guest to
> figure whenever it's autorepeat or really multiple key presses.

Makes sense. It would be nice to have a comment somewhere
(include file near the prototype for the 'deliver key event'
function?) saying this is the required key-repeat behaviour.

I wouldn't be totally surprised if some of our UIs weren't
getting this right, but I'm not sure I care enough to audit
them all :-)

Anyway, I think this reinforces Markus's conclusion that
TigerVNC is correct and our VNC server implementation is
correct and the bug is in whichever PV guest is not
handling the key-repeat info it gets out of PS/2. (At least
I assume that the Xen config in question is going to send
key events via emulated PS/2; if it's something else I
guess that something else could potentially be buggy.)

-- PMM

[Qemu-devel] 答复: [PATCH] vnc: add additional key up event before repeated key down

[Li, Guang]

> -----邮件原件-----
> 发件人: qemu-devel-bounces+lig.fnst=cn.fujitsu.com@nongnu.org
> [mailto:qemu-devel-bounces+lig.fnst=cn.fujitsu.com@nongnu.org] 代表 Peter
> Maydell
> 发送时间: 2014年9月18日 0:09
> 收件人: Gerd Hoffmann
> 抄送: Stefano Stabellini; Markus Armbruster; qemu-devel;
> xen-devel@lists.xen.org; Anthony Liguori; Chun Yan Liu
> 主题: Re: [Qemu-devel] [PATCH] vnc: add additional key up event before
> repeated key down
> 
> On 17 September 2014 04:24, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >> Which brings us to the other half of this: what does our UI layer
> >> specify should be the behaviour for key repeat?
> >> Gerd, can you clarify what the common input layer's expectation is
> >> here? Should UI front ends call qemu_input_event_send_key() with
> >> 'down/down/down/up' or 'down/up/down/up' semantics?
> >
> > It isn't formally specified anywhere.  The UIs usually simply pass
> > through the key events they get.  IMO it is more useful to go for
> > down/down/down/up.  This is how PS/2 works, and this allows the guest
> > to figure whenever it's autorepeat or really multiple key presses.
> 
> Makes sense. It would be nice to have a comment somewhere (include file near
> the prototype for the 'deliver key event'
> function?) saying this is the required key-repeat behaviour.
> 
> I wouldn't be totally surprised if some of our UIs weren't getting this right, but
> I'm not sure I care enough to audit them all :-)
> 
> Anyway, I think this reinforces Markus's conclusion that TigerVNC is correct
> and our VNC server implementation is correct and the bug is in whichever PV
> guest is not handling the key-repeat info it gets out of PS/2. (At least I assume
> that the Xen config in question is going to send key events via emulated PS/2; if
> it's something else I guess that something else could potentially be buggy.)

Confirmed, we surely do right things now to collect make,break keycode and handle repeat case,
But, seems forgot to handle typematic.

Thanks!