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

["Chun Yan Liu" <cyliu@xxxxxxxx>]

>>> On 9/6/2014 at 05:23 AM, in message
<alpine.DEB.2.02.1409052202451.2334@xxxxxxxxxxxxxxxxxxxxxxx>, Stefano
Stabellini <stefano.stabellini@xxxxxxxxxxxxx> 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@xxxxxxxx> 
> > --- 
> >  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 
> >  
>  
>  


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel