Re: [Qemu-devel] [PATCH] SDL mouse events smoothness

2008-02-26 Thread Johannes Schindelin 
Hi,

On Tue, 26 Feb 2008, Samuel Thibault wrote:

 I asked on the SDL mailing list, and they answered that qemu should 
 indeed not use SDL_GetRelativeMouseState(), since that only provides the 
 latest mouse position, not the position at the time of the event.

AFAIR this is done so that your guest system does not spend ages 
processing mouse events from 10 minutes ago.  This could happen easily if 
your guest is on high load.

IOW I am not quite sure if I want the behaviour your patch provides, so if 
that patch should be taken, there should be at least an option to turn 
that behaviour off.

Ciao,
Dscho

Re: [Qemu-devel] [PATCH] SDL mouse events smoothness

2008-02-26 Thread Samuel Thibault 
Johannes Schindelin, le Tue 26 Feb 2008 12:57:25 +, a écrit :
 On Tue, 26 Feb 2008, Samuel Thibault wrote:
 
  I asked on the SDL mailing list, and they answered that qemu should 
  indeed not use SDL_GetRelativeMouseState(), since that only provides the 
  latest mouse position, not the position at the time of the event.
 
 AFAIR this is done so that your guest system does not spend ages 
 processing mouse events from 10 minutes ago.  This could happen easily if 
 your guest is on high load.

Mmm, but these events currently _are_ sent to the guest...  Events could
be coalesced indeed (and an option provided to choose whether to do it
or not), but that's not currently the case.

 IOW I am not quite sure if I want the behaviour your patch provides, so if 
 that patch should be taken, there should be at least an option to turn 
 that behaviour off.

Well, at least compared to the current situation (a lot of undeeded
events pushed to the guest), my patch is just an improvement, because at
least these events are now useful.

Samuel

[Qemu-devel] [PATCH] SDL mouse events smoothness

2008-02-26 Thread Samuel Thibault 
Hello,

I was having a look at the mouse events that the guest receives, and was
surprised to get

pos x:452 y:220 z:0 
pos x:452 y:220 z:0 
pos x:452 y:220 z:0 
pos x:452 y:220 z:0 
pos x:452 y:220 z:0 
pos x:430 y:304 z:0 
pos x:430 y:304 z:0 
pos x:430 y:304 z:0 
pos x:430 y:304 z:0 
pos x:430 y:304 z:0 
pos x:430 y:304 z:0 
pos x:430 y:304 z:0 
pos x:430 y:304 z:0 
pos x:430 y:304 z:0 
pos x:350 y:344 z:0 
pos x:350 y:344 z:0 
pos x:350 y:344 z:0  

i.e. the guest receives the same position several times.  This revealed
to be because sdl.c uses SDL_GetRelativeMouseState() instead of just
using the value from the event, and thus when several X11 events are
processed in one execution of sdl_refresh(), the intermediate positions
are not taken into account.  I asked on the SDL mailing list, and they
answered that qemu should indeed not use SDL_GetRelativeMouseState(),
since that only provides the latest mouse position, not the position at
the time of the event.  The patch below fixes this, and now I am getting

pos x:401 y:457 z:0 
pos x:393 y:461 z:0 
pos x:387 y:465 z:0 
pos x:384 y:465 z:0 
pos x:378 y:469 z:0 
pos x:378 y:470 z:0 
pos x:379 y:472 z:0 
pos x:387 y:478 z:0 
pos x:385 y:494 z:0 
pos x:381 y:512 z:0

which provides a much more smooth feedback of the mouse cursor.  It also
permits accessibility features such as mouse trail to work much better.
It also fixes some of the double-clic issues noticed when the machine is
sluggish (since it now always uses the button state at the time of the
event, not the current button state).

Samuel

Index: sdl.c
===
RCS file: /sources/qemu/qemu/sdl.c,v
retrieving revision 1.45
diff -u -p -r1.45 sdl.c
--- sdl.c   17 Nov 2007 17:14:38 -  1.45
+++ sdl.c   26 Feb 2008 12:24:33 -
@@ -276,8 +276,6 @@ static void sdl_grab_start(void)
 } else
 sdl_hide_cursor();
 SDL_WM_GrabInput(SDL_GRAB_ON);
-/* dummy read to avoid moving the mouse */
-SDL_GetRelativeMouseState(NULL, NULL);
 gui_grab = 1;
 sdl_update_caption();
 }
@@ -290,10 +288,9 @@ static void sdl_grab_end(void)
 sdl_update_caption();
 }
 
-static void sdl_send_mouse_event(int dz)
+static void sdl_send_mouse_event(int dx, int dy, int dz, int x, int y, int 
state)
 {
-int dx, dy, state, buttons;
-state = SDL_GetRelativeMouseState(dx, dy);
+int buttons;
 buttons = 0;
 if (state  SDL_BUTTON(SDL_BUTTON_LEFT))
 buttons |= MOUSE_EVENT_LBUTTON;
@@ -311,18 +308,18 @@ static void sdl_send_mouse_event(int dz)
absolute_enabled = 1;
}
 
-   SDL_GetMouseState(dx, dy);
-   dx = dx * 0x7FFF / width;
-   dy = dy * 0x7FFF / height;
+   dx = x * 0x7FFF / width;
+   dy = y * 0x7FFF / height;
 } else if (absolute_enabled) {
sdl_show_cursor();
absolute_enabled = 0;
 } else if (guest_cursor) {
-SDL_GetMouseState(dx, dy);
-dx -= guest_x;
-dy -= guest_y;
-guest_x += dx;
-guest_y += dy;
+x -= guest_x;
+y -= guest_y;
+guest_x += x;
+guest_y += y;
+dx = x;
+dy = y;
 }
 
 kbd_mouse_event(dx, dy, dz, buttons);
@@ -347,6 +344,7 @@ static void sdl_refresh(DisplayState *ds
 {
 SDL_Event ev1, *ev = ev1;
 int mod_state;
+int buttonstate = SDL_GetMouseState(NULL, NULL);
 
 if (last_vm_running != vm_running) {
 last_vm_running =