Skip to contentSkip to conversation

mouse click simple queue

PATCH In review Stefano Stabellini 3 participants 5 messages Jan 28, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mouse click simple queue

Hi all,

qemu doesn't enqueue mouse events, just records the latest mouse state. This can cause some lost mouse double clicks if the events are not processed fast enought. I am attaching a patch that implements a simple queue for left mouse click events.

Best Regards,

Stefano Stabellini

Index: sdl.c
===================================================================
RCS file: /sources/qemu/qemu/sdl.c,v
retrieving revision 1.45
diff -r1.45 sdl.c
293c293
< static void sdl_send_mouse_event(int dz)
---

Quoted 1 line

static void sdl_send_mouse_event(int dx, int dy, int dz, int state)

295,297c295,296

<     int dx, dy, state, buttons;
<     state = SDL_GetRelativeMouseState(&dx, &dy);
<     buttons = 0;

---

Quoted 2 lines
    int buttons = 0;

314c313
< SDL_GetMouseState(&dx, &dy);
---

Quoted 1 line
        SDL_GetMouseState(&dx, &dy);

477c476,478
< sdl_send_mouse_event(0);
---

Quoted 3 lines
                int dx, dy, state;
                state = SDL_GetRelativeMouseState(&dx, &dy);
                sdl_send_mouse_event(dx, dy, 0, state);

480d480
< case SDL_MOUSEBUTTONDOWN:
481a482,488

Quoted 7 lines
            if (gui_grab || kbd_mouse_is_absolute()) {
                int dx, dy, state;
                state = SDL_GetRelativeMouseState(&dx, &dy);
                sdl_send_mouse_event(dx, dy, 0, state);
            }
            break;
        case SDL_MOUSEBUTTONDOWN:

491c498
< int dz;
---

Quoted 1 line
                    int dx, dy, dz, state;

492a500

Quoted 1 line
                    state = SDL_GetRelativeMouseState(&dx, &dy);

494c502
< if (bev->button == SDL_BUTTON_WHEELUP && ev->type == SDL_MOUSEBUTTONDOWN) {
---

Quoted 1 line
                    if (bev->button == SDL_BUTTON_WHEELUP) {

496c504,505
< } else if (bev->button == SDL_BUTTON_WHEELDOWN && ev->type == SDL_MOUSEBUTTONDOWN) {
---

Quoted 2 lines
                    }
                    else if (bev->button == SDL_BUTTON_WHEELDOWN) {

498a508,510

Quoted 3 lines
                    else {
                        state = bev->button | state;
                    }

500c512
< sdl_send_mouse_event(dz);
---

Quoted 1 line
                    sdl_send_mouse_event(dx, dy, dz, state);

Index: hw/usb-hid.c
===================================================================
RCS file: /sources/qemu/qemu/hw/usb-hid.c,v
retrieving revision 1.16
diff -r1.16 usb-hid.c
386a387,395

Quoted 9 lines

static int currentbutton = 0;

typedef struct _mouseclick {
    int button_state;
    struct _mouseclick *next;
} mouseclick; 

static mouseclick mousequeue[20];
static mouseclick *head = mousequeue;
static mouseclick *tail = mousequeue;

392a402,414

Quoted 4 lines
    if (hs->changed == 1){
        //A mouse event is lost
        if (buttons_state != currentbutton && tail->next != head) {
            //A left click event is lost: let's add it to the queue
Quoted 9 lines
            //counter++;
            tail->button_state = buttons_state;
            tail = tail->next;
        }
    }
    else {
        s->buttons_state = buttons_state;
    }

396d417
< s->buttons_state = buttons_state;
397a419

Quoted 1 line
    currentbutton = buttons_state;

405a428,440

Quoted 4 lines
    if (hs->changed == 1){
        //A mouse event is lost
        if (buttons_state != currentbutton && tail->next != head) {
            //A left click event is lost: let's add it to the queue
Quoted 9 lines
            //counter++;
            tail->button_state = buttons_state;
            tail = tail->next;
        }
    }
    else {
        s->buttons_state = buttons_state;
    }

409d443
< s->buttons_state = buttons_state;
410a445

Quoted 1 line
    currentbutton = buttons_state;

779c814
< s->changed = 0;
---

Quoted 1 line

785a821,833

Quoted 13 lines
            if ((s->kind == USB_MOUSE || s->kind == USB_TABLET) && s->changed) {
                USBMouseState *ms = &s->ptr;
                if (head != tail) {
                    ms->buttons_state = head->button_state;
                    head = head->next;
                }
                else {
                    s->changed = 0;
                }
            } else {
                s->changed = 0;
            }

810a859

Quoted 1 line
    int i;

812a862,868

Quoted 7 lines
    for (i = 0; i < 19; i++) {
        mousequeue[i].button_state = 0;
        mousequeue[i].next = &(mousequeue[i + 1]);
    }
    mousequeue[i].button_state = 0;
    mousequeue[i].next = mousequeue;

833a890

Quoted 1 line
    int i;

835a893,899

Quoted 7 lines
    for (i = 0; i < 19; i++) {
        mousequeue[i].button_state = 0;
        mousequeue[i].next = &(mousequeue[i + 1]);
    }
    mousequeue[i].button_state = 0;
    mousequeue[i].next = mousequeue;
#1 lore raw

Sorry for the format of the patch, this time I have generated it using diff -uNp.

Quoted 10 lines · Stefano Stabellini

Hi all,

qemu doesn't enqueue mouse events, just records the latest mouse state. This can cause some lost mouse double clicks if the events are not processed fast enought. I am attaching a patch that implements a simple queue for left mouse click events.

Best Regards,

Stefano Stabellini

Index: sdl.c
===================================================================
RCS file: /sources/qemu/qemu/sdl.c,v
retrieving revision 1.45

sdl.c +23 -11
diff -u -p -r1.45 sdl.c
@@ -290,11 +290,10 @@ 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);
-    buttons = 0;
+    int buttons = 0;
+    
     if (state & SDL_BUTTON(SDL_BUTTON_LEFT))
         buttons |= MOUSE_EVENT_LBUTTON;
     if (state & SDL_BUTTON(SDL_BUTTON_RIGHT))
@@ -311,7 +310,7 @@ static void sdl_send_mouse_event(int dz)
 	    absolute_enabled = 1;
 	}
 
-	SDL_GetMouseState(&dx, &dy);
+        SDL_GetMouseState(&dx, &dy);
 	dx = dx * 0x7FFF / width;
 	dy = dy * 0x7FFF / height;
     } else if (absolute_enabled) {
@@ -474,11 +473,19 @@ 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, state;
+                state = SDL_GetRelativeMouseState(&dx, &dy);
+                sdl_send_mouse_event(dx, dy, 0, state);
             }
             break;
-        case SDL_MOUSEBUTTONDOWN:
         case SDL_MOUSEBUTTONUP:
+            if (gui_grab || kbd_mouse_is_absolute()) {
+                int dx, dy, state;
+                state = SDL_GetRelativeMouseState(&dx, &dy);
+                sdl_send_mouse_event(dx, dy, 0, state);
+            }
+            break;
+        case SDL_MOUSEBUTTONDOWN:
             {
                 SDL_MouseButtonEvent *bev = &ev->button;
                 if (!gui_grab && !kbd_mouse_is_absolute()) {
@@ -488,16 +495,21 @@ static void sdl_refresh(DisplayState *ds
                         sdl_grab_start();
                     }
                 } else {
-                    int dz;
+                    int dx, dy, dz, state;
                     dz = 0;
+                    state = SDL_GetRelativeMouseState(&dx, &dy);
 #ifdef SDL_BUTTON_WHEELUP
-                    if (bev->button == SDL_BUTTON_WHEELUP && ev->type == SDL_MOUSEBUTTONDOWN) {
+                    if (bev->button == SDL_BUTTON_WHEELUP) {
                         dz = -1;
-                    } else if (bev->button == SDL_BUTTON_WHEELDOWN && ev->type == SDL_MOUSEBUTTONDOWN) {
+                    }
+                    else if (bev->button == SDL_BUTTON_WHEELDOWN) {
                         dz = 1;
                     }
+                    else {
+                        state = bev->button | state;
+                    }
 #endif
-                    sdl_send_mouse_event(dz);
+                    sdl_send_mouse_event(dx, dy, dz, state);
                 }
             }
             break;

Index: hw/usb-hid.c
===================================================================
RCS file: /sources/qemu/qemu/hw/usb-hid.c,v
retrieving revision 1.16

usb-hid.c +67 -3

124 lines not on this page · raw message

#2 lore raw
In reply to Stefano Stabellini
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line · Stefano Stabellini

Stefano> Hi all, qemu doesn't enqueue mouse events, just records the Stefano> latest mouse state. This can cause some lost mouse double Stefano> clicks if the events are not processed fast enought. I am Stefano> attaching a patch that implements a simple queue for left Stefano> mouse click events.

Good point! :-)

But is it possible to use higher-level queue constructions rather than inlining the queue behaviour in the code? Because it will be hard to maintain if everybody introduces her own queue, list or whatever structure each time it is needed...

It reminds me the MINIX source code, quite hard to modify because of this kind of stuff... :-)

Thank you!

Signature · 5 lines
  Ronan KERYELL                 |\/  Tel:    (+33|0) 2.29.00.14.15
  Département Informatique      |/)  Fax:    (+33|0) 2.29.00.12.82
  TÉLÉCOM Bretagne, CS 83818    K    GSM:    (+33|0) 6.13.14.37.66
  F-29238 PLOUZANÉ CEDEX 3      |\   E-mail: [email protected]
  FRANCE                        | \  http://enstb.org/~keryell
#3 lore raw
Quoted 2 lines · Ronan Keryell, le Mon 28 Jan 2008 13:24:27 +0100, a écrit :

But is it possible to use higher-level queue constructions rather than inlining the queue behaviour in the code?

There is QEMUFIFO code in console.c which could be shared for instance, yes.

Samuel

#4 lore raw
Quoted 7 lines · Samuel Thibault
Quoted 2 lines · Ronan Keryell, le Mon 28 Jan 2008 13:24:27 +0100, a écrit :

But is it possible to use higher-level queue constructions rather than inlining the queue behaviour in the code?

There is QEMUFIFO code in console.c which could be shared for instance, yes.

QEMUFIFO cannot be used directly for the mouse click queue.
What can be done is writing a generic queue library independent from the type of the elements. Then we could use the library wherever needed. If you think that it is a good idea I'll post a patch that implements this kind of library.