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

2008-03-13 Thread Aurelien Jarno
On Wed, Mar 05, 2008 at 01:54:53PM +, Samuel Thibault wrote:
 Here is a revamped patch:
 
 This 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, let's avoid that.
 - 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).
 - bev-state doesn't contain the button state but whether the event is a press
   or a release. Use bev-button instead.

This patch does not apply anymore. Could you please to redo it against
the current CVS?

-- 
  .''`.  Aurelien Jarno | GPG: 1024D/F1BCDB73
 : :' :  Debian developer   | Electrical Engineer
 `. `'   [EMAIL PROTECTED] | [EMAIL PROTECTED]
   `-people.debian.org/~aurel32 | www.aurel32.net




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

2008-03-13 Thread Samuel Thibault
Aurelien Jarno, le Thu 13 Mar 2008 20:50:51 +0100, a écrit :
 On Wed, Mar 05, 2008 at 01:54:53PM +, Samuel Thibault wrote:
  This 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, let's avoid that.
  - 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).
  - bev-state doesn't contain the button state but whether the event is a 
  press
or a release. Use bev-button instead.
 
 This patch does not apply anymore. Could you please to redo it against
 the current CVS?

Mmm, well, it actually is conflicting with the mouse smoothness patch:
the question is: after the 30ms period, if we got several mouse motion
events, should we merge them into a single one for the guest, or should
we provide all of them (hence making the cursor looking more smooth, but
requiring more treatments from the guest)?

Samuel




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

2008-03-05 Thread 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 -  1.45
 +++ sdl.c 5 Mar 2008 11:53:55 -
 @@ -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





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

2008-03-05 Thread Samuel Thibault
Johannes Schindelin, le Wed 05 Mar 2008 14:09:10 +0100, a écrit :
 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?

That's it.

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

Yes, that's what I said in my comments.

   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?

It's actually a bug fix: state doesn't contain the button state but the
0/1 according to the event being a press or release event. Yes, that's
duplicate information from SDL.

Samuel




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

2008-03-05 Thread Samuel Thibault
Here is a revamped patch:

This 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, let's avoid that.
- 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).
- bev-state doesn't contain the button state but whether the event is a press
  or a release. Use bev-button instead.

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   5 Mar 2008 11:53:55 -
@@ -290,10 +290,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 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;
@@ -347,6 +346,7 @@ static void sdl_refresh(DisplayState *ds
 {
 SDL_Event ev1, *ev = ev1;
 int mod_state;
+int button_state;
 
 if (last_vm_running != vm_running) {
 last_vm_running = vm_running;
@@ -355,6 +355,7 @@ static void sdl_refresh(DisplayState *ds
 
 vga_hw_update();
 
+button_state = SDL_GetMouseState(NULL, NULL);
 while (SDL_PollEvent(ev)) {
 switch (ev-type) {
 case SDL_VIDEOEXPOSE:
@@ -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, button_state);
 }
 break;
 case SDL_MOUSEBUTTONDOWN:
 case SDL_MOUSEBUTTONUP:
 {
 SDL_MouseButtonEvent *bev = ev-button;
+if (ev-type == SDL_MOUSEBUTTONDOWN)
+button_state |= SDL_BUTTON(bev-button);
+else
+button_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)) {
 /* start grabbing all events */
 sdl_grab_start();
 }
@@ -497,7 +505,7 @@ static void sdl_refresh(DisplayState *ds
 dz = 1;
 }
 #endif
-sdl_send_mouse_event(dz);
+sdl_send_mouse_event(0, 0, dz, button_state);
 }
 }
 break;




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

2008-03-05 Thread Johannes Schindelin
Hi,

On Wed, 5 Mar 2008, Samuel Thibault wrote:

 Here is a revamped patch:

Thanks, and thanks for the explanations, too.

Ciao,
Dscho