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

[Luiz Capitulino]

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.