Merge lp:~oif-team/grail/trunk.fixes into lp:grail

[Henrik Rydberg]

A bunch of patches aiming to integrate the ideas of v1.0.19 more seamlessly into the grail code. This will fix the remaining tap bug and help further development. The only part not addressed by this branch is the sysflag, and a discussion around it is likely enough to see how to form a patch for it.

[Stephen M. Webb (bregma)]

A more elegant and nuanced way of handling the various timeout issues. Compiles and runs OK.

review: Approve

[Chase Douglas (chasedouglas)]

In line 371 of the patch below, there's an ambiguity in the if statement "if (a || b && c)". This should be cleaned up.

I need to test this branch, but I think the changes look reasonable. However, I can't approve the request until it also handles the global flag. This is something I think we will really need in natty, since we're going to have more applications using gestures.

review: Needs Fixing

[Chase Douglas (chasedouglas)]

Test results:

1. Regression: If you use a physical trackpad button to click while there's no movement on the trackpad, it's held up until a movement is performed.

2. Timeouts seem ok. I was able to request all 3, 4, and 5 finger gestures and still perform two finger multitouch reasonably. I will perform more thorough testing when the regressions are resolved.

3. Regression: I often get a tap event at the end of a drag gesture sequence. The tap touch count matches the drag touch count.

review: Needs Fixing

[Henrik Rydberg (rydberg)]

Thank you very much for the tests! I have updated the branch with fixes for
the two problems found. I will add something for the global flag shortly.

[lp:~oif-team/grail/trunk.fixes]

115. By Henrik Rydberg on 2011-02-24

Revert "Add button passthrough"

116. By Henrik Rydberg on 2011-02-24

Revert "Add global vs local gesture handling"

117. By Henrik Rydberg on 2011-02-24

Introduce gesture timeouts

The timeout of a gesture is integral to the gesture type.
This patch introduces timeout per gesture type, potentially
enabling more control over gesture timing aspects. The timeout
logic replaces the recently added recognition state.

118. By Henrik Rydberg on 2011-02-24

Introduce activation hold time

Do not activate a drag, pinch or rotate immediately, even
if the activation threshold is met. This allows for a smoother
transition between fingers.

119. By Henrik Rydberg on 2011-02-24

Wait for the right number of fingers before gesture timeout

A client listening only for multi-finger gestures will not get
the chance to recognize unless there is a gesture timeout. This
patch waits until the drag hold time has passed before releasing
the touch.

120. By Henrik Rydberg on 2011-02-24

Skip events selectively

Buffer all events as planned, but strip the stream of all events
involved with pointer and MT, rather than bluntly removing all
events. In practise, this means button and switch events are let
through, undistorted although possibly slightly delayed.

121. By Henrik Rydberg on 2011-02-24

Adjust gesture buffer sizes and timings

Adjust timings to put drag, pinch and rotate on more equal footing,
with regard to gesture timeouts. Also make sure events are not lost
during hold, now that all MT events are buffered as well.

122. By Henrik Rydberg on 2011-02-24

Reset tap state after timeout

A touch and hold will not properly reset the tap state, such that
the next tap fails to register. Fixed with this patch.

123. By Henrik Rydberg on 2011-02-24

Remove unused tapping state

This patch removes some cruft that was left over from the
removal of the short tap time logic.

124. By Henrik Rydberg on 2011-02-24

Make reset logic of pinch and drag same as for drag

There is no direct reason for one-fingered pinch/rotate gestures
to be discarded immediately, so only reset gestures on touch down,
as for drags.

125. By Henrik Rydberg on 2011-03-04

Make grabbing client exclusive

When a possible gesture is started for a global client (sysflag set),
it is desirable that gestures to other clients are disregarded until
all contacts have left the surface. In principle this could be refined
further, such that if gesture are not detected, they are passed on to
other clients, but that also affects how touches are passed onto lower
layers like XI2.1, so simply making sure a global system client gets
all events potentially leading to gestures is good enough for now.

Also note that in the current combination of synaptics/evdev X servers
and grail, there can be no overlapping potential gestures between two
global clients, or the resulting event recipient is not correct.

126. By Henrik Rydberg on 2011-03-04

Bump to v1.0.20

127. By Henrik Rydberg on 2011-03-17

Disable rotation gestures for semi-mt devices

To avoid poor user experience with semi-mt devices, disable
rotation detection, such that only drag and scale is emitted.

Signed-off-by: Chase Douglas <email address hidden>
Signed-off-by: Henrik Rydberg <email address hidden>

Unmerged revisions

=== modified file 'src/evbuf.h'
--- src/evbuf.h	2010-08-06 20:30:20 +0000
+++ src/evbuf.h	2011-02-24 13:47:05 +0000
@@ -29,7 +29,7 @@
 #ifndef GRAIL_EVBUF_H
 #define GRAIL_EVBUF_H
 
-#define DIM_EVENTS 512
+#define DIM_EVENTS 4096
 
 struct evbuf {
 	int head;
 
=== modified file 'src/gestures-drag.c'
--- src/gestures-drag.c	2011-01-02 12:08:08 +0000
+++ src/gestures-drag.c	2011-02-24 13:47:05 +0000
@@ -72,6 +72,13 @@
 			state->active = 0;
 		}
 	}
+	if ((move->timeout & fm_mask) == fm_mask) {
+		if (state->active) {
+			gin_gid_discard(ge, state->gid);
+			state->active = 0;
+		}
+		return 0;
+	}
 	if (!state->active) {
 		int type = getype[move->ntouch];
 		if (type < 0)
@@ -102,6 +109,13 @@
 			state->active = 0;
 		}
 	}
+	if ((move->timeout & fm_mask) == fm_mask) {
+		if (state->active) {
+			gin_gid_discard(ge, state->gid);
+			state->active = 0;
+		}
+		return 0;
+	}
 	if (!state->active) {
 		if (move->ntouch == 4) {
 			state->gid = gin_gid_begin(ge, GRAIL_TYPE_MDRAG,
 
=== modified file 'src/gestures-pinch.c'
--- src/gestures-pinch.c	2011-01-02 12:08:08 +0000
+++ src/gestures-pinch.c	2011-02-24 13:47:05 +0000
@@ -62,6 +62,13 @@
 		}
 		return 0;
 	}
+	if ((move->timeout & fm_mask) == fm_mask) {
+		if (state->active) {
+			gin_gid_discard(ge, state->gid);
+			state->active = 0;
+		}
+		return 0;
+	}
 	if (!(move->tickle & mask))
 		return 0;
 	if (!state->active) {
@@ -93,6 +100,13 @@
 		}
 		return 0;
 	}
+	if ((move->timeout & fm_mask) == fm_mask) {
+		if (state->active) {
+			gin_gid_discard(ge, state->gid);
+			state->active = 0;
+		}
+		return 0;
+	}
 	if (!(move->tickle & mask))
 		return 0;
 	if (!state->active) {
 
=== modified file 'src/gestures-rotate.c'
--- src/gestures-rotate.c	2011-01-02 12:08:08 +0000
+++ src/gestures-rotate.c	2011-02-24 13:47:05 +0000
@@ -61,6 +61,13 @@
 		}
 		return 0;
 	}
+	if ((move->timeout & fm_mask) == fm_mask) {
+		if (state->active) {
+			gin_gid_discard(ge, state->gid);
+			state->active = 0;
+		}
+		return 0;
+	}
 	if (!(move->tickle & mask))
 		return 0;
 	if (!state->active) {
@@ -92,6 +99,13 @@
 		}
 		return 0;
 	}
+	if ((move->timeout & fm_mask) == fm_mask) {
+		if (state->active) {
+			gin_gid_discard(ge, state->gid);
+			state->active = 0;
+		}
+		return 0;
+	}
 	if (!(move->tickle & mask))
 		return 0;
 	if (!state->active) {
 
=== modified file 'src/gestures-tapping.c'
--- src/gestures-tapping.c	2011-02-09 15:17:50 +0000
+++ src/gestures-tapping.c	2011-02-24 13:47:05 +0000
@@ -91,6 +91,8 @@
 	if ((move->active & fm_mask) ||
 	    move->time - state->start > move->fm[FM_X].bar_ms) {
 		gin_gid_discard(ge, state->gid);
+		state->mintouch = 0;
+		state->maxtouch = 0;
 		state->active = 0;
 	}
 	return 0;
 
=== modified file 'src/grail-api.c'
--- src/grail-api.c	2011-02-21 01:22:08 +0000
+++ src/grail-api.c	2011-02-24 13:47:05 +0000
@@ -34,11 +34,6 @@
 #define DIM_FRAMES 100
 #define FRAME_RATE 100
 
-/* Wait time (ms) for number of touches to stabilize. */
-static const unsigned int stable_time = 50;
-/* Maximum time (ms) for gesture to be recognized. */
-static const unsigned int recognition_time = 300;
-
 void grail_filter_abs_events(struct grail *ge, int usage)
 {
 	struct grail_impl *x = ge->impl;
@@ -132,159 +127,103 @@
 	max->y = s->max_y;
 }
 
-/* Check for any potential gestures given the number of touch points and where
- * they occur on screen. */
-static int check_potential_gestures(struct grail *ge,
-				    const struct utouch_frame *frame)
-{
-	grail_mask_t types[DIM_GRAIL_TYPE_BYTES] = {0};
-	struct grail_client_info client;
-	struct grail_coord pos[frame->num_active];
-	int i;
-
-	switch (frame->num_active) {
-		case 1:
-			grail_mask_set(types, GRAIL_TYPE_DRAG1);
-			grail_mask_set(types, GRAIL_TYPE_PINCH1);
-			grail_mask_set(types, GRAIL_TYPE_ROTATE1);
-			grail_mask_set(types, GRAIL_TYPE_TAP1);
-			break;
-		case 2:
-			grail_mask_set(types, GRAIL_TYPE_DRAG2);
-			grail_mask_set(types, GRAIL_TYPE_PINCH2);
-			grail_mask_set(types, GRAIL_TYPE_ROTATE2);
-			grail_mask_set(types, GRAIL_TYPE_TAP2);
-			break;
-		case 3:
-			grail_mask_set(types, GRAIL_TYPE_DRAG3);
-			grail_mask_set(types, GRAIL_TYPE_PINCH3);
-			grail_mask_set(types, GRAIL_TYPE_ROTATE3);
-			grail_mask_set(types, GRAIL_TYPE_TAP3);
-			grail_mask_set(types, GRAIL_TYPE_EDRAG);
-			grail_mask_set(types, GRAIL_TYPE_EPINCH);
-			grail_mask_set(types, GRAIL_TYPE_EROTATE);
-			break;
-		case 4:
-			grail_mask_set(types, GRAIL_TYPE_DRAG4);
-			grail_mask_set(types, GRAIL_TYPE_PINCH4);
-			grail_mask_set(types, GRAIL_TYPE_ROTATE4);
-			grail_mask_set(types, GRAIL_TYPE_TAP4);
-			grail_mask_set(types, GRAIL_TYPE_MDRAG);
-			grail_mask_set(types, GRAIL_TYPE_MPINCH);
-			grail_mask_set(types, GRAIL_TYPE_MROTATE);
-			break;
-		case 5:
-			grail_mask_set(types, GRAIL_TYPE_DRAG5);
-			grail_mask_set(types, GRAIL_TYPE_PINCH5);
-			grail_mask_set(types, GRAIL_TYPE_ROTATE5);
-			grail_mask_set(types, GRAIL_TYPE_TAP5);
-			break;
-	}
-
-	for (i = 0; i < frame->num_active; i++) {
-		pos[i].x = gin_prop_x(ge->gin, frame->active[i]->x);
-		pos[i].y = gin_prop_y(ge->gin, frame->active[i]->y);
-	}
-
-	return ge->get_clients(ge, &client, 1, pos, frame->num_active,
-			       types, DIM_GRAIL_TYPE_BYTES);
+static void flush_events(struct grail *ge)
+{
+	struct grail_impl *impl = ge->impl;
+	struct input_event iev;
+
+	grailbuf_clear(&impl->gbuf);
+	while (!evbuf_empty(&impl->evbuf)) {
+		evbuf_get(&impl->evbuf, &iev);
+		if (ge->event)
+			ge->event(ge, &iev);
+	}
+}
+
+static int skip_event(const struct input_event *ev, int count)
+{
+	switch (ev->type) {
+	case EV_ABS:
+		return 1;
+	case EV_KEY:
+		return ev->code >= BTN_DIGI && ev->code < BTN_WHEEL;
+	case EV_SYN:
+		switch (ev->code) {
+		case SYN_MT_REPORT:
+			return 1;
+		case SYN_REPORT:
+			return count == 0;
+		default:
+			return 0;
+		}
+	default:
+		return 0;
+	}
+}
+
+static void flush_gestures(struct grail *ge)
+{
+	struct grail_impl *impl = ge->impl;
+	struct input_event iev;
+	struct grail_event gev;
+	int count = 0;
+
+	while (!evbuf_empty(&impl->evbuf)) {
+		evbuf_get(&impl->evbuf, &iev);
+		if (skip_event(&iev, count))
+			continue;
+		if (ge->event)
+			ge->event(ge, &iev);
+		if (iev.type == EV_SYN && iev.code == SYN_REPORT)
+			count = 0;
+		else
+			count++;
+	}
+	while (!grailbuf_empty(&impl->gbuf)) {
+		grailbuf_get(&impl->gbuf, &gev);
+		if (ge->gesture)
+			ge->gesture(ge, &gev);
+	}
 }
 
 static void report_frame(struct grail *ge,
 			 const struct utouch_frame *frame,
 			 const struct input_event *syn)
 {
-	struct grail_impl *impl = ge->impl;
 	struct gesture_recognizer *gru = ge->gru;
-	struct input_event iev;
+	struct gesture_inserter *gin = ge->gin;
+	struct grail_impl *impl = ge->impl;
 	struct grail_event gev;
 
 	ge->impl->frame = frame;
 
-	/* Reset timer when number of touches changes. */
-	if (frame->prev->revision != frame->revision && frame->num_active) {
-		gru->start_time = frame->time;
-	}
-
-	/* Once touches have stabilized, check if there are any potential
-	 * gestures registered for the number of touches. */
-	if (gru->state == RECOGNIZING &&
-	    frame->time > gru->start_time + stable_time &&
-	    check_potential_gestures(ge, frame) == 0)
-		gru->state = UNRECOGNIZED;
-
-	/* Process the frame for gestures unless we're sure there are none. */
-	if (gru->state != UNRECOGNIZED) {
-		gin_frame_begin(ge, frame);
-		gru_recognize(ge, frame);
-		gin_frame_end(ge, frame);
-	}
-
-	if (gru->state == RECOGNIZING) {
-		/* If a gesture is recognized and either all touches lifted or
-		 * we are within the recognition interval, set state to
-		 * recognized. Note that taps do not stay in the used array, so
-		 * we must check if any gesture events are buffered. */
-		if ((grail_mask_count(ge->gin->used, DIM_INSTANCE_BYTES) ||
-		     !grailbuf_empty(&impl->gbuf)) &&
-		    (frame->num_active == 0 ||
-		     (frame->time > gru->start_time + stable_time &&
-		      frame->time <= gru->start_time + recognition_time))) {
-			if (gru->global_client)
-				gru->state = GLOBAL;
-			else
-				gru->state = LOCAL;
-		}
-		/* If there is no gesture and all touches lifted, set state to
-		 * unrecognized. */
-		else if (frame->num_active == 0)
-			gru->state = UNRECOGNIZED;
-	}
-
-	/* Now we have the state set, process events. */
-	switch (gru->state) {
-		case LOCAL:
-		case GLOBAL:
-			while (!grailbuf_empty(&impl->gbuf)) {
-				grailbuf_get(&impl->gbuf, &gev);
-				if (ge->gesture &&
-				    ((gru->state == GLOBAL &&
-				      gev.client_id.client == gru->global_client) ||
-				     (gru->state == LOCAL &&
-				      gev.client_id.client != gru->global_client)))
-					ge->gesture(ge, &gev);
-			}
-			evbuf_clear(&impl->evbuf);
-
-			/* Once all touches are lifted and no gestures are
-			 * active, reset recognition state. */
-			if (grail_mask_count(ge->gin->used, DIM_INSTANCE_BYTES) == 0 &&
-			    frame->num_active == 0) {
-				gru->state = RECOGNIZING;
-				gru->global_client = 0;
-			}
-			break;
-
-		case RECOGNIZING:
-			/* If we're still actively recognizing, stop here. */
-			if (frame->time <= gru->start_time + recognition_time)
-				break;
-			gru->state = UNRECOGNIZED;
-
-		case UNRECOGNIZED:
-			while (!evbuf_empty(&impl->evbuf)) {
-				evbuf_get(&impl->evbuf, &iev);
-				if (ge->event)
-					ge->event(ge, &iev);
-			}
-			grailbuf_clear(&impl->gbuf);
-
-			if (frame->num_active == 0) {
-				gru->state = RECOGNIZING;
-				gru->global_client = 0;
-			}
-			break;
-	}
+	if (frame->num_active && !frame->prev->num_active) {
+		impl->timeout = 0;
+		impl->gesture = 0;
+	}
+
+	if (impl->timeout) {
+		flush_events(ge);
+		return;
+	}
+
+	gin_frame_begin(ge, frame);
+	gru_recognize(ge, frame);
+	gin_frame_end(ge, frame);
+
+	if (!grailbuf_empty(&impl->gbuf))
+		impl->gesture = 1;
+
+	if (!frame->num_active ||
+	    !grail_mask_count(gin->used, sizeof(gin->used)) &&
+	    frame->time - frame->mod_time > gru->move.fm[FM_X].hold_ms)
+		impl->timeout = !impl->gesture;
+
+	if (impl->timeout)
+		flush_events(ge);
+
+	if (impl->gesture)
+		flush_gestures(ge);
 }
 
 static void grail_pump_mtdev(struct grail *ge, const struct input_event *ev)
@@ -292,21 +231,6 @@
 	struct grail_impl *impl = ge->impl;
 	const struct utouch_frame *frame;
 
-	if (ev->type == EV_KEY && (ev->code < BTN_DIGI ||
-                                   ev->code > BTN_TOOL_QUADTAP)) {
-		struct input_event sync_ev;
-
-		ge->event(ge, ev);
-
-		sync_ev.type = EV_SYN;
-		sync_ev.code = SYN_REPORT;
-		sync_ev.value = 1;
-		sync_ev.time = ev->time;
-		ge->event(ge, &sync_ev);
-
-		return;
-	}
-
 	evbuf_put(&impl->evbuf, ev);
 
 	if (ev->type == EV_SYN || ev->type == EV_ABS) {
 
=== modified file 'src/grail-event.c'
--- src/grail-event.c	2011-02-09 15:12:13 +0000
+++ src/grail-event.c	2011-02-24 13:47:05 +0000
@@ -22,7 +22,6 @@
 
 #include "grail-inserter.h"
 #include "grail-impl.h"
-#include "grail-recognizer.h"
 #include <malloc.h>
 #include <string.h>
 #include <errno.h>
@@ -83,7 +82,7 @@
 		    const struct utouch_frame *frame)
 {
 	struct grail_coord pos[DIM_TOUCH];
-	int i, npos = 0, num_clients;
+	int i, npos = 0;
 	if (!ge->get_clients)
 		return 0;
 	grail_mask_foreach(i, span, bspan) {
@@ -91,20 +90,7 @@
 		pos[npos].y = gin_prop_y(ge->gin, frame->slots[i]->y);
 		npos++;
 	}
-
-	num_clients = ge->get_clients(ge, info, maxinfo, pos, npos, types,
-				      btypes);
-
-	/* Check if client is handling global gestures, using SYSFLAG1. */
-	if (num_clients > 0 && grail_mask_get(info[0].mask,
-					      GRAIL_TYPE_SYSFLAG1)) {
-		/* Save client id, and clear SYSFLAG1 as it isn't a real
-		 * gesture type. */
-		ge->gru->global_client = info[0].id.client;
-		grail_mask_clear(info[0].mask, GRAIL_TYPE_SYSFLAG1);
-	}
-
-	return num_clients;
+	return ge->get_clients(ge, info, maxinfo, pos, npos, types, btypes);
 }
 
 void gin_send_event(struct grail *ge, struct slot_state *s,
 
=== modified file 'src/grail-gestures.c'
--- src/grail-gestures.c	2011-01-02 12:08:08 +0000
+++ src/grail-gestures.c	2011-02-24 13:47:05 +0000
@@ -24,9 +24,10 @@
 #include "grail-impl.h"
 #include <math.h>
 
-static const float FM_SN[DIM_FM] = { 1000, 1000, 200, 1000 };
-static const float FM_BAR[DIM_FM] = { 50, 50, 30, 50 };
-static const grail_time_t FM_BAR_MS[DIM_FM] = { 300, 300, 10000, 10000 };
+static const float FM_SN[DIM_FM] = { 1000, 1000, 1000, 1000 };
+static const float FM_BAR[DIM_FM] = { 50, 50, 50, 50 };
+static const grail_time_t FM_HOLD_MS[DIM_FM] = { 60, 60, 60, 60 };
+static const grail_time_t FM_BAR_MS[DIM_FM] = { 300, 300, 500, 500 };
 static const grail_time_t SAMPLE_MS = 10;
 static const float EPS = 1e-3;
 
@@ -111,6 +112,7 @@
 	fm->sample_ms = t;
 	m->tickle &= ~(1 << i);
 	m->active &= ~(1 << i);
+	m->timeout &= ~(1 << i);
 }
 
 static void move_update(struct move_model *m, int i, float x, grail_time_t t)
@@ -132,11 +134,15 @@
 	if (m->active & (1 << i))
 		return;
 	fm->action_delta = x - fm->original;
-	if (fabs(fm->action_delta) > fm->bar ||
-	    t - fm->original_ms > fm->bar_ms)
-		m->active |= (1 << i);
-	else
+	if (t - fm->original_ms > fm->hold_ms &&
+	    fabs(fm->action_delta) > fm->bar) {
+		m->active |= (1 << i);
+	} else if (t - fm->original_ms > fm->bar_ms) {
+		m->active |= (1 << i);
+		m->timeout |= (1 << i);
+	} else {
 		fm->action_delta = 0;
+	}
 }
 
 void gru_init_motion(struct grail *ge)
@@ -153,6 +159,7 @@
 	for (i = 0; i < DIM_FM; i++) {
 		m->fm[i].fuzz = D[i] / FM_SN[i];
 		m->fm[i].bar = D[i] / FM_BAR[i];
+		m->fm[i].hold_ms = FM_HOLD_MS[i];
 		m->fm[i].bar_ms = FM_BAR_MS[i];
 	}
 }
 
=== modified file 'src/grail-gestures.h'
--- src/grail-gestures.h	2011-01-02 12:08:08 +0000
+++ src/grail-gestures.h	2011-02-24 13:47:05 +0000
@@ -49,12 +49,13 @@
 	float bar;
 	grail_time_t original_ms;
 	grail_time_t sample_ms;
+	grail_time_t hold_ms;
 	grail_time_t bar_ms;
 };
 
 struct move_model {
 	struct filter_model fm[DIM_FM];
-	int tickle, active;
+	int tickle, active, timeout;
 	int single, multi, ntouch;
 	grail_time_t time;
 };
 
=== modified file 'src/grail-impl.h'
--- src/grail-impl.h	2011-02-02 22:16:20 +0000
+++ src/grail-impl.h	2011-02-24 13:47:05 +0000
@@ -42,6 +42,8 @@
 	struct evbuf evbuf;
 	struct grailbuf gbuf;
 	int filter_abs;
+	int timeout;
+	int gesture;
 };
 
 #endif
 
=== modified file 'src/grail-recognizer.h'
--- src/grail-recognizer.h	2011-02-09 15:12:13 +0000
+++ src/grail-recognizer.h	2011-02-24 13:47:05 +0000
@@ -25,13 +25,6 @@
 
 #include "grail-gestures.h"
 
-enum recognition_state {
-	RECOGNIZING,
-	LOCAL,
-	GLOBAL,
-	UNRECOGNIZED
-};
-
 struct gesture_recognizer {
 	struct move_model move;
 	struct combo_model drag;
@@ -41,9 +34,6 @@
 	struct combo_model winpinch;
 	struct combo_model winrotate;
 	struct tapping_model tapping;
-	utouch_frame_time_t start_time;
-	enum recognition_state state;
-	int global_client;
 };
 
 int gru_init(struct grail *ge);