On Sun, Apr 29, 2012 at 07:42:53PM +0200, Raphaël Droz wrote:
> On Sun, Apr 29, 2012 at 03:52:07PM +0200, Thorsten Wißmann wrote:
>
> > I also fixed the annoying lag between a click and the reaction by abook.
> > Now the mouse should just work as expected™. Can someone test it and
> > give feedback? (Then we could enable it by default)
>
> One visual problem I found is that editing a field immediately makes
> some garbage appear in the bottom editing bar ( #X* ).
Thanks for fixing that.
> I also found some problem with wheel-down handling. It appears that
> ncurses before 2005 didn't support it (at least it didn't #define
> corresponding events). Then it comes for --with-ext-mouse ncurses build
> (which is still not the default for gentoo in 5.9), see:
> $ grep -m1 NCURSES_MOUSE_VERSION /usr/include/ncursesw/curses.h
>
> AFAICT button2 represents the middle click rather than the wheel but the
> patch appeared to work for me because I'm using a newer version of
> rxvt-unicode which embed built-in support for mouse wheel.
>
> The attached patch should apply on top of yours.
I've also changed your patch, added it to my "Fix lag..."-commit, pushed
it and attached it to this message.
> * It does not attach to BUTTON1_PRESS event anymore as it's not needed,
> unless I missed something.
I personally prefer the selection on BUTTON1_PRESS rather than on
BUTTON1_CLICKED (press + release), because it feels faster and because
there's no need to wait for the mouse button to be released.
> * Conditionally set mousemask() according to ncurses headers and removed
> the use of BUTTON2_PRESS which AFAICT is not involved in mouse-wheel-down,
> whatever NCURSES_MOUSE_VERSION is.
> There may be more elegant function to do this in the ncurses API but I
> didn't take the time to look at them.
>
> * detach from mouse signal before we start editing a field (in
> edit_field()) and restore before we return.
>
> * removed some debug statements
>
> So it's a bit crappy (4x #if NCURSES_MOUSE_VERSION) but tries to play
> nice with ncurses headers. Let me know if you have any idea in order to
> make the whole better.
I reduced that to 2x: once in ui_enable_mouse(bool) and once in ui.h
where I define BUTTON5_PRESSED for old ncurses versions. But i *DON'T*
have any test-system with an old enough ncurses version.
> Finally, I've a bad feeling about was_double_click() / custom handling
> of mouse interval ? do we really need this ? isn't what
> mouseinterval(200) would do ?
No, because mouseinterval(200) buffers the click event for 200ms. I.e.
it buffers a single click event until it definitely can tell it can't be
a double-click. But that's only important if there's a difference
between "single click on item X and then double-click on X" and "only
double-click on X". In our case there's no difference so we don't want
to wait for 200ms for the single click event.
> It may also be a bug in ncurses, or a bad use of the mouse handling API ?
> Could you ensure that this is the right way to fix the lag (I didn't
> find myself the UI laggy) ?
I couldn't find documentation about the "right way" and I don't know any
other ncurses-application that handles double-clicks.
> or that it still happens when we stop listening
> to BUTTON1_PRESSED ? ...
If we stop listening to BUTTON1_PRESS, the lag still is there.
> PS: I also attached the colors-related patches rebased/merged, as they
> will appear in the upstream/master repository once pushed; unless you have
> some
> additional remarks. (I would personally have preferred to see 2/2
> Fabio's patch merged into Thorsten's one but either way wouldn't hurt).
Feel free to push them upstream.
Thorsten
>From 4f1d5792f32ffa2387a8e47c615f8ddf98956b3f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thorsten=20Wi=C3=9Fmann?= <e...@thorsten-wissmann.de>
Date: Sun, 29 Apr 2012 15:14:21 +0200
Subject: [PATCH] Fix the lag between a click and KEY_MOUSE-event
Now, all events for clicks and scroll actions are received immediately.
So abook has to recognize double clicks itself by measuring the time
between two clicks.
---
edit.c | 20 ++++++++++----------
ui.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
ui.h | 4 ++++
3 files changed, 54 insertions(+), 17 deletions(-)
diff --git a/edit.c b/edit.c
index 5f794a8..16503cf 100644
--- a/edit.c
+++ b/edit.c
@@ -588,13 +588,14 @@ key_to_field_number(char c)
static void
edit_field(int tab, char c, int item_number)
{
+ ui_enable_mouse(FALSE);
int i = 0, number, idx;
char *msg;
abook_field_list *f;
list_item item;
if((number = key_to_field_number(c)) < 0)
- return;
+ goto detachfield;
edit_undo(item_number, BACKUP_ITEM);
@@ -602,7 +603,7 @@ edit_field(int tab, char c, int item_number)
while(1) {
if(!f)
- return;
+ goto detachfield;
if(i == number)
break;
@@ -631,10 +632,13 @@ edit_field(int tab, char c, int item_number)
break;
case FIELD_DATE:
edit_date(item_number, idx);
- return;
+ goto detachfield;
default:
assert(0);
}
+
+ detachfield:
+ ui_enable_mouse(TRUE);
}
static int
@@ -671,13 +675,10 @@ edit_loop(int item)
if(c == KEY_MOUSE) {
MEVENT event;
if(getmouse(&event) == OK) {
- if(event.bstate & BUTTON1_PRESSED
- || event.bstate & BUTTON1_CLICKED
+ if(event.bstate & BUTTON1_CLICKED
|| event.bstate & BUTTON1_DOUBLE_CLICKED) {
int window_y, window_x;
getbegyx(editw, window_y, window_x);
- /* fprintf(stderr, "tabline at %d\n",
TABLINE); */
- /* fprintf(stderr, "mouse at %d\n",
event.y+window_y); */
if(event.y == 0) {
/* if first row is selected, then go
back to list */
return -1;
@@ -711,10 +712,9 @@ edit_loop(int item)
}
} else if(event.bstate & BUTTON4_PRESSED) {
tab = tab == 0 ? views_count - 1 : tab - 1;
- } else if(event.bstate & 0x80 || event.bstate &
0x8000000) {
- /* scroll wheel up --> but which ncurses
#define? */
+ }
+ else if(event.bstate & BUTTON5_PRESSED) {
tab = tab == views_count - 1 ? 0 : tab + 1;
- } else {
}
return item;
}
diff --git a/ui.c b/ui.c
index 5cc2087..590e2cc 100644
--- a/ui.c
+++ b/ui.c
@@ -26,6 +26,7 @@
#include "filter.h"
#include "xmalloc.h"
#include "color.h"
+#include <sys/time.h>
#ifdef HAVE_CONFIG_H
# include "config.h"
#endif
@@ -51,6 +52,8 @@ static bool ui_initialized = FALSE;
static bool should_resize = FALSE;
static bool can_resize = FALSE;
+static struct timeval last_click_time;
+static int double_click_interval = 200; /* maximum time in milliseconds */
static WINDOW *top = NULL, *bottom = NULL;
@@ -131,7 +134,9 @@ ui_init_curses()
nonl();
intrflush(stdscr, FALSE);
if(opt_get_bool(BOOL_USE_MOUSE)) {
- mousemask(ALL_MOUSE_EVENTS, NULL);
+ mouseinterval(0);
+ timerclear(&last_click_time);
+ ui_enable_mouse(TRUE);
}
keypad(stdscr, TRUE);
if(opt_get_bool(BOOL_USE_COLORS)) {
@@ -141,6 +146,35 @@ ui_init_curses()
}
}
+void
+ui_enable_mouse(bool enabled)
+{
+ mmask_t mask;
+ if(enabled) {
+ mask = BUTTON1_CLICKED | BUTTON4_PRESSED;
+#if NCURSES_MOUSE_VERSION == 2
+ mask |= BUTTON5_PRESSED;
+#endif
+ } else {
+ mask = 0;
+ }
+ mousemask(mask, NULL);
+}
+
+/** Check the time elapsed since last click and tell if it should be
+ * interpreted as a double click
+ */
+static bool
+was_double_click() {
+ struct timeval click_time, click_diff, maxdiff;
+ maxdiff.tv_sec = double_click_interval / 1000;
+ maxdiff.tv_usec = (double_click_interval % 1000)*1000;
+ gettimeofday(&click_time, NULL);
+
+ timersub(&click_time, &last_click_time, &click_diff);
+ last_click_time = click_time;
+ return !timercmp(&click_diff, &maxdiff, >);
+}
#define CHECK_COLOR_NAME(value, name, DEFNAME) \
if(!strcmp((name), (value))){ \
@@ -508,23 +542,22 @@ get_commands()
can_resize = FALSE; /* it's not safe to resize anymore */
if(ch == KEY_MOUSE) {
MEVENT event;
+ bool double_clicked = was_double_click();
if(getmouse(&event) == OK) {
- if(event.bstate & BUTTON1_PRESSED
- || event.bstate & BUTTON1_CLICKED
+ if(event.bstate & BUTTON1_CLICKED
|| event.bstate & BUTTON1_DOUBLE_CLICKED) {
if(event.y == 0) {
return;
}
list_set_curitem(event.y +
list_get_firstitem() - LIST_TOP);
- if(event.bstate &
BUTTON1_DOUBLE_CLICKED) {
+ if(double_clicked) {
edit_item(-1);
} else {
refresh_list();
}
- } else if(event.bstate & BUTTON4_PRESSED
- || event.bstate & BUTTON4_CLICKED) {
+ } else if(event.bstate & BUTTON4_PRESSED) {
scroll_up();
- } else if(event.bstate & 0x80 || event.bstate &
0x8000000) {
+ } else if(event.bstate & BUTTON5_PRESSED) {
scroll_down();
}
}
diff --git a/ui.h b/ui.h
index fbd5a8b..b94bffe 100644
--- a/ui.h
+++ b/ui.h
@@ -11,6 +11,7 @@ enum {
int is_ui_initialized();
void ui_init_curses();
void ui_init_color_pairs_user();
+void ui_enable_mouse(bool enabled);
int init_ui();
void close_ui();
void headerline(const char *str);
@@ -38,6 +39,9 @@ char *get_surname(char *s);
void ui_print_database();
void ui_open_datafile();
+#if NCURSES_MOUSE_VERSION != 2
+#define BUTTON5_PRESSED (0x80 | 0x8000000)
+#endif
#include "options.h" /* needed for options_get_bool */
--
1.7.10
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Abook-devel mailing list
Abook-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/abook-devel
