[7/9] spice: add mouse

Open mouse channel.  Now you can move the guests mouse pointer.
No tablet / absolute positioning (yet) though.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 spice-input.c |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)

On 08/19/2010 07:40 AM, Gerd Hoffmann wrote:
> Open mouse channel.  Now you can move the guests mouse pointer.
> No tablet / absolute positioning (yet) though.
>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
> ---
>   spice-input.c |   31 +++++++++++++++++++++++++++++++
>   1 files changed, 31 insertions(+), 0 deletions(-)
>
> diff --git a/spice-input.c b/spice-input.c
> index e1014d7..8f3deb4 100644
> --- a/spice-input.c
> +++ b/spice-input.c
> @@ -46,12 +46,43 @@ static void kbd_leds(void *opaque, int ledstate)
>       spice_server_kbd_leds(&kbd->sin, ledstate);
>   }
>
> +/* mouse bits */
> +
> +typedef struct QemuSpiceMouse {
> +    SpiceMouseInstance sin;
> +} QemuSpiceMouse;
> +
> +static void mouse_motion(SpiceMouseInstance *sin, int dx, int dy, int dz,
> +                         uint32_t buttons_state)
> +{
> +    kbd_mouse_event(dx, dy, dz, buttons_state);
>    

dz is an odd interface.  We use it to represent additional buttons which 
really makes no sense.  If you still can, I'd suggest moving dz into 
buttons_state.

Previous comment still applies though, you should explicitly convert the 
button_states from QEMU format to Spice format to future proof.

Regards,

Anthony Liguori

> +}
> +
> +static void mouse_buttons(SpiceMouseInstance *sin, uint32_t buttons_state)
> +{
> +    kbd_mouse_event(0, 0, 0, buttons_state);
> +}
> +
> +static const SpiceMouseInterface mouse_interface = {
> +    .base.type          = SPICE_INTERFACE_MOUSE,
> +    .base.description   = "mouse",
> +    .base.major_version = SPICE_INTERFACE_MOUSE_MAJOR,
> +    .base.minor_version = SPICE_INTERFACE_MOUSE_MINOR,
> +    .motion             = mouse_motion,
> +    .buttons            = mouse_buttons,
> +};
> +
>   void qemu_spice_input_init(void)
>   {
>       QemuSpiceKbd *kbd;
> +    QemuSpiceMouse *mouse;
>
>       kbd = qemu_mallocz(sizeof(*kbd));
>       kbd->sin.base.sif =&kbd_interface.base;
>       spice_server_add_interface(spice_server,&kbd->sin.base);
>       qemu_add_led_event_handler(kbd_leds, kbd);
> +
> +    mouse = qemu_mallocz(sizeof(*mouse));
> +    mouse->sin.base.sif =&mouse_interface.base;
> +    spice_server_add_interface(spice_server,&mouse->sin.base);
>   }
>

Hi,

>> +static void mouse_motion(SpiceMouseInstance *sin, int dx, int dy, int
>> dz,
>> + uint32_t buttons_state)
>> +{
>> + kbd_mouse_event(dx, dy, dz, buttons_state);
>
> dz is an odd interface. We use it to represent additional buttons which
> really makes no sense. If you still can, I'd suggest moving dz into
> buttons_state.

Again this is libspice interface (and I think also wire protocol), so I 
can't change it.  I can convert dz into some button_mask bits before 
calling kbd_mouse_event, but looking at the vnc code it seems qemu 
expects the mouse wheel events being passed via dz not button_state.

> Previous comment still applies though, you should explicitly convert the
> button_states from QEMU format to Spice format to future proof.

Ok.

cheers,
   Gerd

On 08/20/2010 07:42 AM, Gerd Hoffmann wrote:
>   Hi,
>
>>> +static void mouse_motion(SpiceMouseInstance *sin, int dx, int dy, int
>>> dz,
>>> + uint32_t buttons_state)
>>> +{
>>> + kbd_mouse_event(dx, dy, dz, buttons_state);
>>
>> dz is an odd interface. We use it to represent additional buttons which
>> really makes no sense. If you still can, I'd suggest moving dz into
>> buttons_state.
>
> Again this is libspice interface (and I think also wire protocol), so 
> I can't change it.  I can convert dz into some button_mask bits before 
> calling kbd_mouse_event, but looking at the vnc code it seems qemu 
> expects the mouse wheel events being passed via dz not button_state.

That's unfortunate but understood.  Is Spice considered a stable 
API/wire protocol at this point?

Regards,

Anthony Liguori

>
>> Previous comment still applies though, you should explicitly convert the
>> button_states from QEMU format to Spice format to future proof.
>
> Ok.
>
> cheers,
>   Gerd
>

Hi,

>> Again this is libspice interface (and I think also wire protocol), so
>> I can't change it. I can convert dz into some button_mask bits before
>> calling kbd_mouse_event, but looking at the vnc code it seems qemu
>> expects the mouse wheel events being passed via dz not button_state.
>
> That's unfortunate but understood. Is Spice considered a stable API/wire
> protocol at this point?

Yes.  There are version numbers though, so we can add new messages and 
use them in case both server and client are new enough.  Unfortunaly 
this makes the keyboard f*ckup only slightly less messy as we'll have to 
support both old+new way for compatibility reasons.  Oh well.

cheers,
   Gerd

On 08/20/2010 09:03 AM, Gerd Hoffmann wrote:
>   Hi,
>
>>> Again this is libspice interface (and I think also wire protocol), so
>>> I can't change it. I can convert dz into some button_mask bits before
>>> calling kbd_mouse_event, but looking at the vnc code it seems qemu
>>> expects the mouse wheel events being passed via dz not button_state.
>>
>> That's unfortunate but understood. Is Spice considered a stable API/wire
>> protocol at this point?
>
> Yes.  There are version numbers though, so we can add new messages and 
> use them in case both server and client are new enough.  Unfortunaly 
> this makes the keyboard f*ckup only slightly less messy as we'll have 
> to support both old+new way for compatibility reasons.  Oh well.

One of the things to consider is that we only emulate a 2-axis 5 button 
mouse.  Buttons one through three are bits in button_state and buttons 
4/5 are represented through dz.  My mouse IRL is actually a 9 button mouse.

We encode buttons 4/5 as a -1, 0, 1 in dz which corresponds to 10, 00, 
and 11.  We cannot represent 11 with dz which corresponds to having 
buttons 4 and 5 pressed simultaneously.  Most mice have a scroll wheel 
for 4/5 but there's no strict requirement that that's the case.

So in terms of building a long term stable wire protocol, you'd be much 
better off using button_mask to encode buttons 4/5 because it's just a 
matter of time before we drop the dz hack in qemu.

Regards,

Anthony Liguori

> cheers,
>   Gerd
>