From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33771) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uo3pA-0003Na-Tb for qemu-devel@nongnu.org; Sat, 15 Jun 2013 23:40:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Uo3p8-0005LP-7X for qemu-devel@nongnu.org; Sat, 15 Jun 2013 23:40:16 -0400 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Sun, 16 Jun 2013 05:39:57 +0200 Message-Id: <1371354005-26873-1-git-send-email-afaerber@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] [PATCH RFC 0/8] monitor: Fix mouse_button and improve mouse_move commands List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: "Bernhard M. Wiedemann" , xen-devel , Stephan Kulow , Brad Hards , Peter Maydell , Stefano Stabellini , Mark Cave-Ayland , Alexander Graf , Luiz Capitulino , Blue Swirl , qemu-ppc , Gerd Hoffmann , Paolo Bonzini , Ludwig Nussel , =?UTF-8?q?Andreas=20F=C3=A4rber?= Hi everyone, Here's a new stab at fixing mouse_button HMP command with absolute coordi= nates, as seen with the following devices: usb-tablet, usb-wacom-tablet, ads7846= and depending on settings vmmouse and xenfb, as well as pre-qdev tsc2005, tsc= 2102, tsc2301 touchscreens. openQA [3] is a framework for recording input sequences and expected grap= hical guest output. An absolute coordinate system is handy there. :) When sending mouse_button command though, QEMU generates the event at coordinates (0,0), i.e. top left corner for absolute coordinates, so that= a sequence of (qemu) mouse_move 42 42 (qemu) mouse_button 1 may lead the guest to recognize a mouse drag from (42,42) to (0,0) rather than a click at (42,42). Since there can be more than one mouse (and there is for -device usb-tabl= et without -no-defaults) and since there are multiple ways to interact with = the mouse beyond the monitor, using a single global state in the monitor is u= gly. We therefore need to push the event handling to ui/input.c, where the mouse_handlers list (QEMUPutMouseEntry) is accessible. My first attempt was to add coordinates state to QEMUPutMouseEntry, so th= at we can safely switch via mouse_set between absolute and non-absolute mice= . The downside of that approach was that state at that level is not migrata= ble, i.e. we would still warp to (0,0) on mouse_button after migration, and it= is not being reset either. My solution is to obtain position and buttons state from where the VMStat= e is: I clean up the mouse event handler registration code a bit and extend it: * one mandatory callback to obtain mouse button state and * one optional callback to obtain absolute position state from backends. Since I didn't know most mice implementations, these could use some revie= w. * For the msmouse chardev backend I needed to persist buttons state along= side the CharDriverState (no device) because it just wrote it to serial. Should we turn this backend into a device to reset this state? * It seemed that escc is not fully migration-ready (e.g., SerIOQueue and keyboard state missing) so I did not bother to add VMSD fields or a subse= ction for the fields added. Similarly it just wrote it to said SerIOQueue and get_queue() would've destructively read from it. * xenfb stored buttons state but not coordinates, so added them to XenInp= ut state. No VMStateDescription at all here. * hid and vmmouse read the position from some queue (vmmouse also button = state), not sure how safe that is and whether we may need to add it to state dire= ctly? Ludwig, can you test these with the openQA qemu package please? Thanks! Regards, Andreas >>From Brad Hards' mouse_button patch: * Instead of adding more global monitor-only state, delegate to ui/input.= c (suggested by Gerd [1] and Paolo [2]). But instead of keeping state just in ui/input.c:QEMUPutMouseEntry, delegate it to the individual mouse event handlers. * Prepend some code cleanups. * Introduce MouseOps for callbacks (inspired by MemoryRegionOps). * Implement MouseOps::get_position() for all absolute mice. * Implement MouseOps::get_buttons_state() for all mice. * Improve mouse_move while at it (suggested by Gerd to Brad [1]). [1] http://lists.gnu.org/archive/html/qemu-devel/2011-04/msg02408.html [2] https://bugs.launchpad.net/qemu/+bug/752476 [3] http://openqa.opensuse.org/ Cc: Gerd Hoffmann Cc: Paolo Bonzini Cc: Luiz Capitulino Cc: Bernhard M. Wiedemann Cc: Ludwig Nussel Cc: Stephan Kulow Cc: Brad Hards Cc: Blue Swirl (escc) Cc: Mark Cave-Ayland (escc) Cc: Alexander Graf (adb) Cc: qemu-ppc (adb) Cc: Andrzej Zaborowski (tsc2005, tsc210x, ads7846) Cc: Peter Maydell (tsc2005, tsc210x, ads7846) Cc: Stefano Stabellini (xenfb) Cc: xen-devel (xenfb) Andreas F=C3=A4rber (8): ui/input: Clean up QEMUPutMouseEntry struct ui/input: Simplify kbd_mouse_event() ui/input: Use bool for qemu_add_mouse_event_handler() ui/input: Introduce MouseOps for qemu_add_mouse_event_handler() ui/input: Introduce MouseOps::get_buttons_state() monitor: Eliminate global mouse buttons state for mouse_move ui/input: Introduce MouseOps::get_position() monitor: Fix mouse_button command for absolute coordinates backends/msmouse.c | 29 ++++++++++++--- hw/char/escc.c | 17 ++++++++- hw/display/ads7846.c | 25 +++++++++++-- hw/display/xenfb.c | 49 +++++++++++++++++++++++--- hw/input/adb.c | 13 ++++++- hw/input/hid.c | 51 ++++++++++++++++++++++++--- hw/input/ps2.c | 14 +++++++- hw/input/tsc2005.c | 25 +++++++++++-- hw/input/tsc210x.c | 29 ++++++++++++--- hw/input/vmmouse.c | 33 ++++++++++++++++-- hw/usb/dev-wacom.c | 34 +++++++++++++++--- include/ui/console.h | 27 +++++++++++--- monitor.c | 10 +++--- ui/input.c | 99 ++++++++++++++++++++++++++++++++++------------= ------ 14 files changed, 380 insertions(+), 75 deletions(-) --=20 1.8.1.4