Re: [Qemu-devel] [PATCH] monitor: avoid moving cursor during "mouse_button" command

[Gerd Hoffmann]

On 04/08/11 18:37, Luiz Capitulino wrote:
> On Fri, 08 Apr 2011 16:34:21 +0200
> Markus Armbruster<address@hidden>  wrote:
>
> > Brad Hards<address@hidden>  writes:
> >
> > > This addresses https://bugs.launchpad.net/qemu/+bug/752476 which
> > > basically points out that using the mouse_button command causes
> > > the mouse cursor to warp to the origin (when using absolute
> > > pointing device).
> > >
> > > I've tested this with a kubuntu 10.10 guest and it works fine
> > > for me with both relative and absolute pointing devices. Note
> > > that testing with realtive pointing device was relatively
> > > light.
> > >
> > > Signed-off-by: Brad Hards<address@hidden>
> > > ---
> > >   monitor.c |   14 +++++++++++++-
> > >   1 files changed, 13 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/monitor.c b/monitor.c
> > > index f1a08dc..0ce162b 100644
> > > --- a/monitor.c
> > > +++ b/monitor.c
> > > @@ -1879,6 +1879,9 @@ static void do_sendkey(Monitor *mon, const QDict *qdict)
> > >                      muldiv64(get_ticks_per_sec(), hold_time, 1000));
> > >   }
> > >
> > > +static int mouse_x;
> > > +static int mouse_y;
> > > +static int mouse_z;
> > >   static int mouse_button_state;
> > >
> > >   static void do_mouse_move(Monitor *mon, const QDict *qdict)
> > > @@ -1893,13 +1896,22 @@ static void do_mouse_move(Monitor *mon, const QDict 
> > > *qdict)
> > >       if (dz_str)
> > >           dz = strtol(dz_str, NULL, 0);
> > >       kbd_mouse_event(dx, dy, dz, mouse_button_state);
> > > +    if (kbd_mouse_is_absolute()) {
> > > +        mouse_x = dx;
> > > +        mouse_y = dy;
> > > +        mouse_z = dz;
> > > +    }
> > >   }
> > >
> > >   static void do_mouse_button(Monitor *mon, const QDict *qdict)
> > >   {
> > >       int button_state = qdict_get_int(qdict, "button_state");
> > >       mouse_button_state = button_state;
> > > -    kbd_mouse_event(0, 0, 0, mouse_button_state);
> > > +    if (kbd_mouse_is_absolute()) {
> > > +        kbd_mouse_event(mouse_x, mouse_y, mouse_z, mouse_button_state);
> > > +    } else {
> > > +        kbd_mouse_event(0, 0, 0, mouse_button_state);
> > > +    }
> > >   }
> > >
> > >   static void do_ioport_read(Monitor *mon, const QDict *qdict)
> >
> > There's one instance of state: position (if absolute) + buttons for any
> > number of mice.  Funny things can happen when you have more than one
> > mouse and switch between them.
> >
> > Even if there's just one mouse: the state is updated only for monitor
> > mouse action.  Funny things can happen when something other than monitor
> > commands uses the mouse.
> >
> > Shouldn't the state be kept per-mouse?  Monitor could ask for current
> > coordinates + button state then.
> >
> > Note buttons are already funny.  The patch just extends the funniness to
> > position.  Could be a valid excuse for committing it as is.
>
> I need Gerd's input here, or anyone who has a better idea of the trade offs
> involved and how this code should evolve.

I think it would be much better to keep track of the mouse position (and 
button state while being at it) in input.c instead of monitor.c.

Once this is in place it should be easy to add kbd_mouse_* functions 
which update position or buttons only, which the monitor code can use 
then to avoid the unwanted pointer warp.

cheers,
  Gerd