[Spice-devel] [patch 0/2] vdagent KEYVAL extension

[Marc-André Lureau]

----- Original Message -----
> > I still have to answer myself the question "why is the current XT scancode
> > input
> > not enough"? Although I think I have guessed, it would be good to explain
> > that,
> > to motivate the reason for this change.
> 
> Some SPICE applications want to use the keymap from the client side, and
> works directly with UTF input characters (for example a terminal emulator -
> spiceterm).
> The current scancode values cannot be used for those applications.
> 
> Note: you cannot translate a scancode to utf8 (because you do not know the
> keymap of the client).

Or perhaps you could, that would allow to sync client and guest with the same keymap.

> Hope that is clear now?

It's not about utf, you are not using utf encoding. It's about UI "keysyms" or "keyval".

> > > > bypassing all of server, x & qemu & kernel etc...
> > > >
> > > > Arbitrary utf8 input can only be handled at user space / agent level
> > > > (no way to pass a X11/gdk or utf8 through hw level)
> > > >
> > > > And as of today, all agents messages go through the main channel. So
> > > > it is quite normal to recommend to use the main channel.
> > > >
> > > > Now that I review your patch and look at spiceterm usage, I
> > > > understand you are not just passing utf8 input, but you also want
> > > > regular key events, thus the synchronization question arise.
> > > >
> > > > > IMHO, extending the input channel keyboard message format would be
> > > > > the right thing to do. Simply send:
> > > > >
> > > > > - scancode
> > > > > - keysym
> > > > > - modifier key state
> > > > >
> > > > > inside a single message.
> > > >
> > > > That might be a good option too, but that won't be that easy for the
> > > > spice server / agent to deal with. And I also think it is useless to
> > > > send a single message with all values when clearly only one of the 2
> > > > is being used.
> > >
> > > Ok (although I think such separation only makes thing unnecessarily
> > > complex).
> > 
> > For keyboard events generated at human speed, that's not important. I can
> > agree to stick gdk/x11 keysyms in existing messages, when the server has
> > SPICE_INPUTS_CAP_KEY_KEYVAL. But that's not what you proposed in your last
> > patches.
> 
> I thought we cannot modify existing message formats - or is that possible?

It's possible, just like the VD_AGENT_CAP_CLIPBOARD_SELECTION you asked last time.

>  
> > > > I am still suggesting adding a utf8 input message (on input
> > > > channel), synchronized with other existing XT key events (which
> > > > don't have unicode representation).
> > >
> > > I already sent such patch 2 weeks ago - please can you review?
> > >
> > > http://lists.freedesktop.org/archives/spice-devel/2013-September/01434
> > > 2.html
> > > http://lists.freedesktop.org/archives/spice-devel/2013-September/01433
> > > 9.html
> > > http://lists.freedesktop.org/archives/spice-devel/2013-September/01434
> > > 1.html
> > > http://lists.freedesktop.org/archives/spice-devel/2013-September/01434
> > > 0.html
> > 
> > You didn't explain why you needed to add a new message, and a new DOWN/UP
> > flag.
> 
> I want to send keysyms. The DOWN/UP flag is required because keysyms does not
> include that information.
> 
> Note: The Input channel use 3 different message type to indicate
> UP/DOWN/DOWN_AND_UP for scancodes.
> I thought it is easier to use a flag instead.
> 
> > How to interpret scancode with the flag? You can't stick 2 3-bytes
> > scancodes for DOWN/UP, so is the server supposed to generate the UP
> > scancode? (that would be different from existing scancode events)
> 
> Not sure what you talk about - My patch does not modify the behavior for
> scancodes.
> It just send an additional message for the keysym (there is no scancode
> included).

> 
>      message {
>        uint32 keyval;
>        keyboard_keyval_flags flags; # DOWN/UP/DOWN_AND_UP
>     } @ctype(SpiceMsgcKeyKeyval) key_keyval;
> 


That was a previous mistake then:
http://lists.freedesktop.org/archives/spice-devel/2013-September/014342.html


     message {
       uint32 keyval;
       uint32 scancode;
       keyboard_keyval_flags flags;
    } @ctype(SpiceMsgcKeyKeyval) key_keyval;

> > The spice server can't advertize support for this message by default, it
> > needs to
> > be enabled only if the server make use of it. So there need to be an API to
> > change server caps (perhaps there is one already).
> 
> yes, we need to add that.
> 
> > In your server patch, you are not using the scancode.
> 
> yes, I only need the keysyms.
> 
> >So is the client supposed to send both KeyDown/Up and
> > Keyval messages? Then why have scancode sent 2 times?
> > 
> > Perhaps you shouldn't add scancode in your message, and just have gdk/x11
> > keysyms then?
> 
> You want to suppress normal scancode messages to avoid traffic?

That was not my point. But since you removed it, I don't need to discuss this again.

>   
> > Shouldn't be all renamed from "keyval" to "gdk_keyval"?
> 
> I will name it whatever you like.

It's not about what I like, it's about being correct. You call it utf when it has very little to do with utf. You don't explicitely give the keyval encoding, when elsewhere you just assume it is gdk/x11. This cannot be left to the client to decide. You must define it clearly. That's why I dislike just saying gdk/x11, because I am not sure we can just assume they won't change it for example. I would prefer if we stick to unicode (and utf8 representation)

> 
> It is still not clear to me what patch do you prefer - vdagent protocol
> extension or the input channel extension?

In your case, a gdk_keyval message, the input channel.