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

[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 -0000      1.45
+++ sdl.c       26 Feb 2008 12:24:33 -0000
@@ -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 = vm_running;
@@ -474,7 +472,8 @@ static void sdl_refresh(DisplayState *ds
         case SDL_MOUSEMOTION:
             if (gui_grab || kbd_mouse_is_absolute() ||
                 absolute_enabled) {
-                sdl_send_mouse_event(0);
+                sdl_send_mouse_event(ev->motion.xrel, ev->motion.yrel, 0,
+                       ev->motion.x, ev->motion.y, ev->motion.state);
             }
             break;
         case SDL_MOUSEBUTTONDOWN:
@@ -483,13 +482,18 @@ static void sdl_refresh(DisplayState *ds
                 SDL_MouseButtonEvent *bev = &ev->button;
                 if (!gui_grab && !kbd_mouse_is_absolute()) {
                     if (ev->type == SDL_MOUSEBUTTONDOWN &&
-                        (bev->state & SDL_BUTTON_LMASK)) {
+                        (bev->button == SDL_BUTTON_LEFT)) {
                         /* start grabbing all events */
                         sdl_grab_start();
                     }
                 } else {
                     int dz;
                     dz = 0;
+                    if (ev->type == SDL_MOUSEBUTTONDOWN) {
+                        buttonstate |= SDL_BUTTON(bev->button);
+                    } else {
+                        buttonstate &= ~SDL_BUTTON(bev->button);
+                    }
 #ifdef SDL_BUTTON_WHEELUP
                     if (bev->button == SDL_BUTTON_WHEELUP && ev->type == 
SDL_MOUSEBUTTONDOWN) {
                         dz = -1;
@@ -497,7 +501,7 @@ static void sdl_refresh(DisplayState *ds
                         dz = 1;
                     }
 #endif
-                    sdl_send_mouse_event(dz);
+                    sdl_send_mouse_event(0, 0, dz, bev->x, bev->y, 
buttonstate);
                 }
             }
             break;