bug-ncurses
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Mouse button handling


From: Damien Guibouret
Subject: Re: Mouse button handling
Date: Thu, 08 Sep 2011 00:56:23 +0200
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.8) Gecko/20050416

Hello,

Here is the updated diff that adds the mouse event queue compaction to what I already sent. In addition to that, getmouse checks if event is in requested mask or not as there could be cases where KEY_MOUSE is returned without the _nc_mouse_parse function being called (if _nc_mouse_inline returns FALSE). If it is not in the mask it checks previous event until getting a requested one or an invalid event (in this last case it returns ERR). I modified also the test/ncurses.c also to dump the whole queue when a mouse key is detected.

The major impact I see is that the getmouse() will return ERR when an event is not in mask whence it returns OK with a not expected event type previously.

This diff is against the 5.9 original version (I could send a diff against the previous diff if needed).

Regards,

Damien
diff -ur ncurses-5.9.orig/ncurses/base/lib_mouse.c 
ncurses-5.9/ncurses/base/lib_mouse.c
--- ncurses-5.9.orig/ncurses/base/lib_mouse.c   2011-01-22 20:47:47.000000000 
+0100
+++ ncurses-5.9/ncurses/base/lib_mouse.c        2011-09-07 23:36:59.564331384 
+0200
@@ -351,6 +351,8 @@
 #ifndef USE_TERM_DRIVER
 #define xterm_kmous "\033[M"
 
+static mmask_t pressedButtons = 0;
+
 static void
 init_xterm_mouse(SCREEN *sp)
 {
@@ -750,6 +752,7 @@
                        eventp->bstate |= BUTTON3_RELEASED;
                    break;
                default:
+                 eventp->bstate |= REPORT_MOUSE_POSITION;
                    break;
                }
 
@@ -900,30 +903,46 @@
 
 #if USE_EMX_MOUSE
 #define PRESS_POSITION(n) \
-       eventp->bstate = MASK_PRESS(n); \
-       if (kbuf[0] & 0x40) \
-           eventp->bstate = MASK_RELEASE(n)
+       do { \
+               eventp->bstate = MASK_PRESS(n); \
+               pressedButtons |= MASK_PRESS(n); \
+               if (kbuf[0] & 0x40) { \
+                       eventp->bstate = MASK_RELEASE(n); \
+                       pressedButtons &= ~MASK_PRESS(n); \
+               } \
+       } while (0)
 #else
 #define PRESS_POSITION(n) \
-       eventp->bstate = (mmask_t) (prev & MASK_PRESS(n) \
-                                   ? REPORT_MOUSE_POSITION \
-                                   : MASK_PRESS(n))
+       do { \
+               eventp->bstate = (mmask_t) (pressedButtons & MASK_PRESS(n) \
+                                       ? REPORT_MOUSE_POSITION \
+                                       : MASK_PRESS(n)); \
+               pressedButtons |= MASK_PRESS(n); \
+       } while (0)
 #endif
 
        switch (kbuf[0] & 0x3) {
        case 0x0:
-           if (kbuf[0] & 64)
+           if ((kbuf[0] & 96) == 96) {
                eventp->bstate = MASK_PRESS(4);
+               /* Do not memorize this into pressedButtons as there is no 
corresponding
+                * release event */
+           }
            else
                PRESS_POSITION(1);
            break;
 
        case 0x1:
+           if ((kbuf[0] & 96) == 96) {
 #if NCURSES_MOUSE_VERSION == 2
-           if (kbuf[0] & 64)
                eventp->bstate = MASK_PRESS(5);
-           else
+               /* See comment above for button 4 */
+#else
+               /* Ignore this event as it is not a true press of the button */
+               eventp->bstate = REPORT_MOUSE_POSITION;
 #endif
+           }
+           else
                PRESS_POSITION(2);
            break;
 
@@ -939,12 +958,13 @@
             * release, we can infer the button actually released by looking at
             * the previous event.
             */
-           if (prev & (BUTTON_PRESSED | BUTTON_RELEASED)) {
+           if (pressedButtons & BUTTON_PRESSED) {
                eventp->bstate = BUTTON_RELEASED;
                for (b = 1; b <= MAX_BUTTONS; ++b) {
-                   if (!(prev & MASK_PRESS(b)))
+                   if (!(pressedButtons & MASK_PRESS(b)))
                        eventp->bstate &= ~MASK_RELEASE(b);
                }
+               pressedButtons = 0;
            } else {
                /*
                 * XFree86 xterm will return a stream of release-events to
@@ -994,7 +1014,7 @@
        return;
 
     if (on) {
-
+       pressedButtons = 0;
        switch (sp->_mouse_type) {
        case M_XTERM:
 #if NCURSES_EXT_FUNCS
@@ -1074,10 +1094,13 @@
 /* parse a run of atomic mouse events into a gesture */
 {
     MEVENT *eventp = sp->_mouse_eventp;
-    MEVENT *ep, *runp, *next, *prev = PREV(eventp);
+    MEVENT *next, *ep = PREV(eventp);
+    MEVENT *first_valid = NULL;
+    MEVENT *first_invalid = NULL;
     int n;
     int b;
     bool merge;
+    bool endLoop;
 
     TR(MY_TRACE, ("_nc_mouse_parse(%d) called", runcount));
 
@@ -1094,7 +1117,8 @@
      *
      * It's possible that the run may not resolve to a single event (for
      * example, if the user quadruple-clicks).  If so, leading events
-     * in the run are ignored.
+     * in the run are ignored if user does not call getmouse in a loop (getting
+     * them from newest to older).
      *
      * Note that this routine is independent of the format of the specific
      * format of the pointing-device's reports.  We can use it to parse
@@ -1102,79 +1126,105 @@
      * button basis, as long as the device-dependent mouse code puts stuff
      * on the queue in MEVENT format.
      */
-    if (runcount == 1) {
-       TR(MY_TRACE,
-          ("_nc_mouse_parse: returning simple mouse event %s at slot %ld",
-           _nc_tracemouse(sp, prev),
-           (long) IndexEV(sp, prev)));
-       return (prev->id >= NORMAL_EVENT)
-           ? ((prev->bstate & sp->_mouse_mask) ? TRUE : FALSE)
-           : FALSE;
-    }
 
-    /* find the start of the run */
-    runp = eventp;
-    for (n = runcount; n > 0; n--) {
-       runp = PREV(runp);
+    /* Reset all events that were not set just in case user sometimes
+     * calls getmouse only once and other times until there is no more
+     * event in queue.
+     * This allow also reaching the begin of the run. */
+    ep = eventp;
+    for (n = runcount; n < EV_MAX; n++) {
+       ep->id = INVALID_EVENT;
+       ep = NEXT(ep);
     }
 
 #ifdef TRACE
     if (USE_TRACEF(TRACE_IEVENT)) {
        _trace_slot(sp, "before mouse press/release merge:");
        _tracef("_nc_mouse_parse: run starts at %ld, ends at %ld, count %d",
-               RunParams(sp, eventp, runp),
+               RunParams(sp, eventp, ep),
                runcount);
        _nc_unlock_global(tracef);
     }
 #endif /* TRACE */
 
     /* first pass; merge press/release pairs */
-    do {
-       merge = FALSE;
-       for (ep = runp; (next = NEXT(ep)) != eventp; ep = next) {
+    endLoop = FALSE;
+    while (!endLoop) {
+       next = NEXT(ep);
+       if (next == eventp) {
+               /* Will end the loop, but compact before */
+               endLoop = TRUE;
+       } else {
 
 #define MASK_CHANGED(x) (!(ep->bstate & MASK_PRESS(x)) \
                      == !(next->bstate & MASK_RELEASE(x)))
 
-           if (ep->x == next->x && ep->y == next->y
-               && (ep->bstate & BUTTON_PRESSED)
-               && MASK_CHANGED(1)
-               && MASK_CHANGED(2)
-               && MASK_CHANGED(3)
-               && MASK_CHANGED(4)
+               if (ep->id != INVALID_EVENT && next->id != INVALID_EVENT
+                               && ep->x == next->x && ep->y == next->y
+                               && (ep->bstate & BUTTON_PRESSED)
+                               && (!(next->bstate & BUTTON_PRESSED))
+                               && MASK_CHANGED(1)
+                               && MASK_CHANGED(2)
+                               && MASK_CHANGED(3)
+                               && MASK_CHANGED(4)
 #if NCURSES_MOUSE_VERSION == 2
-               && MASK_CHANGED(5)
+                               && MASK_CHANGED(5)
 #endif
-               ) {
-               for (b = 1; b <= MAX_BUTTONS; ++b) {
-                   if ((sp->_mouse_mask & MASK_CLICK(b))
-                       && (ep->bstate & MASK_PRESS(b))) {
-                       ep->bstate &= ~MASK_PRESS(b);
-                       ep->bstate |= MASK_CLICK(b);
-                       merge = TRUE;
-                   }
-               }
-               if (merge)
-                   next->id = INVALID_EVENT;
+               ) {
+                       merge = FALSE;
+                       for (b = 1; b <= MAX_BUTTONS; ++b) {
+                               if ((sp->_mouse_mask & MASK_CLICK(b))
+                                               && (ep->bstate & 
MASK_PRESS(b))) {
+                                       next->bstate &= ~MASK_RELEASE(b);
+                                       next->bstate |= MASK_CLICK(b);
+                                       merge = TRUE;
+                               }
+                       }
+                       if (merge)
+                               ep->id = INVALID_EVENT;
+               }
+       }
+
+           /* Compact valid events */
+           if (ep->id == INVALID_EVENT) {
+               if ((first_valid != NULL) && (first_invalid == NULL)) {
+                       first_invalid = ep;
+               }
+           } else {
+               if (first_valid == NULL) {
+                       first_valid = ep;
+               } else if (first_invalid != NULL) {
+                       *first_invalid = *ep;
+                       ep->id = INVALID_EVENT;
+                       first_invalid = NEXT(first_invalid);
+               }
            }
+
+           ep = next;
+    }
+
+       if (first_invalid != NULL) {
+               eventp = first_invalid;
        }
-    } while
-       (merge);
 
 #ifdef TRACE
     if (USE_TRACEF(TRACE_IEVENT)) {
        _trace_slot(sp, "before mouse click merge:");
-       _tracef("_nc_mouse_parse: run starts at %ld, ends at %ld, count %d",
-               RunParams(sp, eventp, runp),
-               runcount);
-       _nc_unlock_global(tracef);
+       if (first_valid == NULL) {
+               _tracef("_nc_mouse_parse: no valid event");
+       } else {
+               _tracef("_nc_mouse_parse: run starts at %ld, ends at %ld, count 
%d",
+                       RunParams(sp, eventp, first_valid),
+                       runcount);
+               _nc_unlock_global(tracef);
+       }
     }
 #endif /* TRACE */
 
     /*
-     * Second pass; merge click runs.  At this point, click events are
-     * each followed by one invalid event. We merge click events
-     * forward in the queue.
+     * Second pass; merge click runs. We merge click events
+     * forward in the queue such as double click can be changed in triple
+     * click.
      *
      * NOTE: There is a problem with this design!  If the application
      * allows enough click events to pile up in the circular queue so
@@ -1187,87 +1237,95 @@
      * but the timer element would have to have sub-second resolution,
      * which would get us into portability trouble.
      */
-    do {
-       MEVENT *follower;
+       first_invalid = NULL;
+       endLoop = (first_valid == NULL);
+       ep = first_valid;
+       while (!endLoop) {
+               next = NEXT(ep);
+
+               if (next == eventp) {
+               /* Will end the loop, but check event type and compact before */
+                       endLoop = true;
+               } else {
+                       /* merge click events forward */
+                       if ((ep->bstate & BUTTON_CLICKED)
+                                       && (next->bstate & BUTTON_CLICKED)) {
+                               merge = FALSE;
+                               for (b = 1; b <= MAX_BUTTONS; ++b) {
+                                       if ((sp->_mouse_mask & 
MASK_DOUBLE_CLICK(b))
+                                                       && (next->bstate & 
MASK_CLICK(b))) {
+                                               next->bstate &= ~MASK_CLICK(b);
+                                               next->bstate |= 
MASK_DOUBLE_CLICK(b);
+                                               merge = TRUE;
+                                       }
+                               }
+                               if (merge)
+                                       ep->id = INVALID_EVENT;
+                       }
 
-       merge = FALSE;
-       for (ep = runp; (next = NEXT(ep)) != eventp; ep = next)
-           if (ep->id != INVALID_EVENT) {
-               if (next->id != INVALID_EVENT)
-                   continue;
-               follower = NEXT(next);
-               if (follower->id == INVALID_EVENT)
-                   continue;
-
-               /* merge click events forward */
-               if ((ep->bstate & BUTTON_CLICKED)
-                   && (follower->bstate & BUTTON_CLICKED)) {
-                   for (b = 1; b <= MAX_BUTTONS; ++b) {
-                       if ((sp->_mouse_mask & MASK_DOUBLE_CLICK(b))
-                           && (follower->bstate & MASK_CLICK(b))) {
-                           follower->bstate &= ~MASK_CLICK(b);
-                           follower->bstate |= MASK_DOUBLE_CLICK(b);
-                           merge = TRUE;
+                       /* merge double-click events forward */
+                       if ((ep->bstate & BUTTON_DOUBLE_CLICKED)
+                                       && (next->bstate & BUTTON_CLICKED)) {
+                               merge = FALSE;
+                               for (b = 1; b <= MAX_BUTTONS; ++b) {
+                                       if ((sp->_mouse_mask & 
MASK_TRIPLE_CLICK(b))
+                                                       && (next->bstate & 
MASK_CLICK(b))) {
+                                               next->bstate &= ~MASK_CLICK(b);
+                                               next->bstate |= 
MASK_TRIPLE_CLICK(b);
+                                               merge = TRUE;
+                                       }
+                               }
+                               if (merge)
+                                       ep->id = INVALID_EVENT;
                        }
-                   }
-                   if (merge)
+               }
+
+               /* Discard event if it don't match event mask */
+               if (!(ep->bstate & sp->_mouse_mask)) {
                        ep->id = INVALID_EVENT;
                }
 
-               /* merge double-click events forward */
-               if ((ep->bstate & BUTTON_DOUBLE_CLICKED)
-                   && (follower->bstate & BUTTON_CLICKED)) {
-                   for (b = 1; b <= MAX_BUTTONS; ++b) {
-                       if ((sp->_mouse_mask & MASK_TRIPLE_CLICK(b))
-                           && (follower->bstate & MASK_CLICK(b))) {
-                           follower->bstate &= ~MASK_CLICK(b);
-                           follower->bstate |= MASK_TRIPLE_CLICK(b);
-                           merge = TRUE;
+               /* Compact valid events */
+               if (ep->id == INVALID_EVENT) {
+                       if (ep == first_valid) {
+                               first_valid = next;
+                       } else if (first_invalid == NULL) {
+                               first_invalid = ep;
                        }
-                   }
-                   if (merge)
+               } else if (first_invalid != NULL) {
+                       *first_invalid = *ep;
                        ep->id = INVALID_EVENT;
+                       first_invalid = NEXT(first_invalid);
                }
-           }
-    } while
-       (merge);
 
-#ifdef TRACE
-    if (USE_TRACEF(TRACE_IEVENT)) {
-       _trace_slot(sp, "before mouse event queue compaction:");
-       _tracef("_nc_mouse_parse: run starts at %ld, ends at %ld, count %d",
-               RunParams(sp, eventp, runp),
-               runcount);
-       _nc_unlock_global(tracef);
+               ep = next;
+       }
+
+    if (first_invalid == NULL) {
+       first_invalid = eventp;
     }
-#endif /* TRACE */
+    sp->_mouse_eventp = first_invalid;
 
-    /*
-     * Now try to throw away trailing events flagged invalid, or that
-     * don't match the current event mask.
-     */
-    for (; runcount; prev = PREV(eventp), runcount--)
-       if (prev->id == INVALID_EVENT || !(prev->bstate & sp->_mouse_mask)) {
-           sp->_mouse_eventp = eventp = prev;
-       }
 #ifdef TRACE
+       if (first_valid != NULL) {
     if (USE_TRACEF(TRACE_IEVENT)) {
        _trace_slot(sp, "after mouse event queue compaction:");
        _tracef("_nc_mouse_parse: run starts at %ld, ends at %ld, count %d",
-               RunParams(sp, eventp, runp),
+               RunParams(sp, first_invalid, first_valid),
                runcount);
        _nc_unlock_global(tracef);
     }
-    for (ep = runp; ep != eventp; ep = NEXT(ep))
+    for (ep = first_valid; ep != first_invalid; ep = NEXT(ep))
        if (ep->id != INVALID_EVENT)
            TR(MY_TRACE,
               ("_nc_mouse_parse: returning composite mouse event %s at slot 
%ld",
                _nc_tracemouse(sp, ep),
                (long) IndexEV(sp, ep)));
+       }
 #endif /* TRACE */
 
     /* after all this, do we have a valid event? */
-    return (PREV(eventp)->id != INVALID_EVENT);
+    return (PREV(first_invalid)->id != INVALID_EVENT);
 }
 
 static void
@@ -1359,6 +1417,12 @@
        /* compute the current-event pointer */
        MEVENT *prev = PREV(eventp);
 
+       /* Discard events not matching mask (there could be still some in case
+        * _nc_mouse_parse was not called as when _nc_mosue_inline returns 
false) */
+       while ((prev->id != INVALID_EVENT) && (!(prev->bstate & 
SP_PARM->_mouse_mask))) {
+               prev->id = INVALID_EVENT;
+               prev = PREV(prev);
+       }
        if (prev->id != INVALID_EVENT) {
            /* copy the event we find there */
            *aevent = *prev;
@@ -1368,8 +1432,15 @@
                              (long) IndexEV(SP_PARM, prev)));
 
            prev->id = INVALID_EVENT;   /* so the queue slot becomes free */
-           SP_PARM->_mouse_eventp = PREV(prev);
+           SP_PARM->_mouse_eventp = prev;
            result = OK;
+       } else {
+               /* Reset the provided event */
+               aevent->bstate = 0;
+               aevent->id = INVALID_EVENT;
+               aevent->x = 0;
+               aevent->y = 0;
+               aevent->z = 0;
        }
     }
     returnCode(result);
diff -ur ncurses-5.9.orig/test/ncurses.c ncurses-5.9/test/ncurses.c
--- ncurses-5.9.orig/test/ncurses.c     2011-01-22 20:48:33.000000000 +0100
+++ ncurses-5.9/test/ncurses.c  2011-09-07 23:27:50.323828632 +0200
@@ -497,6 +497,18 @@
     refresh();
 }
 
+static void
+wgetch_wrap(WINDOW *win, int first_y)
+{
+    int last_y = getmaxy(win) - 1;
+    int y = getcury(win) + 1;
+
+    if (y >= last_y)
+       y = first_y;
+    wmove(win, y, 0);
+    wclrtoeol(win);
+}
+
 #ifdef NCURSES_MOUSE_VERSION
 /*
  * This function is the same as _tracemouse(), but we cannot count on that
@@ -571,39 +583,50 @@
 }
 
 static void
-show_mouse(WINDOW *win)
+show_mouse(WINDOW *win, int first_y)
 {
     int y, x;
-    MEVENT event;
+    MEVENT events[16];
     bool outside;
     bool show_loc;
+    int nb = 0;
 
-    getmouse(&event);
-    outside = !wenclose(win, event.y, event.x);
-
-    if (outside) {
-       (void) wstandout(win);
-       waddstr(win, "KEY_MOUSE");
-       (void) wstandend(win);
-    } else {
-       waddstr(win, "KEY_MOUSE");
+    while ((nb < 16) && (getmouse(&events[nb]) == OK)) {
+       nb++;
     }
-    wprintw(win, ", %s", mouse_decode(&event));
+    while (nb > 0) {
+       nb--;
+        outside = !wenclose(win, events[nb].y, events[nb].x);
+
+        if (outside) {
+           (void) wstandout(win);
+           waddstr(win, "KEY_MOUSE");
+           (void) wstandend(win);
+        } else {
+           waddstr(win, "KEY_MOUSE");
+        }
+        wprintw(win, ", %s[%d]", mouse_decode(&events[nb]), nb);
+       wclrtoeol(win);
+       if (nb != 0) {
+           wgetch_wrap(win, first_y);
+           waddstr(win, "                  ");
+       }
 
-    if (outside)
-       win = stdscr;
+        if (outside)
+           win = stdscr;
 
-    show_loc = wmouse_trafo(win, &event.y, &event.x, FALSE);
+        show_loc = wmouse_trafo(win, &events[nb].y, &events[nb].x, FALSE);
 
-    if (show_loc) {
-       getyx(win, y, x);
-       wmove(win, event.y, event.x);
-       waddch(win, '*');
-       wmove(win, y, x);
-    }
+        if (show_loc) {
+           getyx(win, y, x);
+           wmove(win, events[nb].y, events[nb].x);
+           waddch(win, '*');
+           wmove(win, y, x);
+        }
 
-    if (outside)
-       wnoutrefresh(win);
+        if (outside)
+           wnoutrefresh(win);
+    }
 }
 #endif /* NCURSES_MOUSE_VERSION */
 
@@ -678,18 +701,6 @@
     wmove(win, y, x);
 }
 
-static void
-wgetch_wrap(WINDOW *win, int first_y)
-{
-    int last_y = getmaxy(win) - 1;
-    int y = getcury(win) + 1;
-
-    if (y >= last_y)
-       y = first_y;
-    wmove(win, y, 0);
-    wclrtoeol(win);
-}
-
 #if defined(KEY_RESIZE) && HAVE_WRESIZE
 typedef struct {
     WINDOW *text;
@@ -873,7 +884,7 @@
            wprintw(win, "Key pressed: %04o ", c);
 #ifdef NCURSES_MOUSE_VERSION
            if (c == KEY_MOUSE) {
-               show_mouse(win);
+               show_mouse(win, first_y);
            } else
 #endif /* NCURSES_MOUSE_VERSION */
            if (c >= KEY_MIN) {
@@ -1135,7 +1146,7 @@
            wprintw(win, "Key pressed: %04o ", (int) c);
 #ifdef NCURSES_MOUSE_VERSION
            if (c == KEY_MOUSE) {
-               show_mouse(win);
+               show_mouse(win, first_y);
            } else
 #endif /* NCURSES_MOUSE_VERSION */
            if (code == KEY_CODE_YES) {

reply via email to

[Prev in Thread] Current Thread [Next in Thread]