Re: [Qemu-devel] [PATCH] fix SDL mouse events processing

[Johannes Schindelin]

Hi,

On Wed, 5 Mar 2008, Samuel Thibault wrote:

> The patch below fixes SDL mouse events processing:
> - GetRelativeMouseState always returns the last position, so when the
>   polling loop gets several mouse events in one go, we would send
>   useless 'no move' events.
> - So as to make sure we don't miss any mouse click / double click, we
>   should not use GetRelativeMouseState() to get the button state, but
>   rather keep records of the button state ourselves (I've requested SDL
>   developers to provide it directly in the event in SDL 1.3).
> 
> 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     5 Mar 2008 11:53:55 -0000
> @@ -355,6 +355,7 @@ static void sdl_refresh(DisplayState *ds
>  
>      vga_hw_update();
>  
> +    state = SDL_GetMouseState(NULL, NULL);

What is this good for?  (I imagine that it would make sense to add a 
comment to document why this is here, for clueless people like me.)

Maybe it is to initialise the state of the mouse buttons?  In that case, 
it might make sense to rename the variable to something less generic.

> @@ -474,16 +475,23 @@ static void sdl_refresh(DisplayState *ds
>          case SDL_MOUSEMOTION:
>              if (gui_grab || kbd_mouse_is_absolute() ||
>                  absolute_enabled) {
> -                sdl_send_mouse_event(0);
> +                int dx, dy;
> +                SDL_GetRelativeMouseState(&dx, &dy);
> +                if (dx || dy)
> +                    sdl_send_mouse_event(dx, dy, 0, state);

This means that you avoid sending (0,0) events.  Good.

>              }
>              break;
>          case SDL_MOUSEBUTTONDOWN:
>          case SDL_MOUSEBUTTONUP:
>              {
>                  SDL_MouseButtonEvent *bev = &ev->button;
> +                if (ev->type == SDL_MOUSEBUTTONDOWN)
> +                    state |= SDL_BUTTON(bev->button);
> +                else
> +                    state &= ~SDL_BUTTON(ev->button.button);
>                  if (!gui_grab && !kbd_mouse_is_absolute()) {
>                      if (ev->type == SDL_MOUSEBUTTONDOWN &&
> -                        (bev->state & SDL_BUTTON_LMASK)) {
> +                        (bev->button == SDL_BUTTON_LEFT)) {

Is this change necessary?

Thanks,
Dscho