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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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

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
> 

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