From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:36135) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QFOk6-0003Qg-A2 for qemu-devel@nongnu.org; Thu, 28 Apr 2011 06:46:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QFOk4-0007ES-Vk for qemu-devel@nongnu.org; Thu, 28 Apr 2011 06:46:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33760) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QFOk4-0007E7-Kv for qemu-devel@nongnu.org; Thu, 28 Apr 2011 06:46:40 -0400 Message-ID: <4DB94581.5080407@redhat.com> Date: Thu, 28 Apr 2011 12:46:25 +0200 From: Gerd Hoffmann MIME-Version: 1.0 References: <1302248640-24913-1-git-send-email-bradh@frogmouth.net> <20110408133708.2a25946a@doriath> In-Reply-To: <20110408133708.2a25946a@doriath> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] monitor: avoid moving cursor during "mouse_button" command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Markus Armbruster , Brad Hards , qemu-devel@nongnu.org On 04/08/11 18:37, Luiz Capitulino wrote: > On Fri, 08 Apr 2011 16:34:21 +0200 > Markus Armbruster wrote: > >> Brad Hards 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 >>> --- >>> 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