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

[Henrik Rydberg]

The changes needed to support non-MT devices through the utouch stack.

[Chase Douglas (chasedouglas)]

Commit 79 makes sense, so no issues there.

However, I'm not sure what to do about commit 80. I think it looks good and it probably can introduce some interesting support for synaptics trackpads. However, it's also really late in the cycle to be adding this much new code.

I know it would be a bit of extra work, but can we keep commit 80 in a separate branch for now and not package it up for Maverick? By strict definition it is clearly out of scope at this point in the development cycle, and it likely won't actually add any new features since synaptics will be used for most of these devices anyways. However, I worry that there could be regressions in the single-touch touchscreen use case.

I guess the most appropriate review state for this is resubmit to include only commit 79. If you aren't meaning to push this into maverick, perhaps this could be merged as 1.1.0 and we would keep maverick on the 1.0.x series that would not include commit 80?

review: Needs Resubmitting

[Henrik Rydberg (rydberg)]

Well, I was thinking maverick for this, since there has already been bug reports about it. I see your point clearly, but I also see the conflict between the normal maverick procedure and the interests to make gestures work for as broad an audience as possible.

[Henrik Rydberg (rydberg)]

Code now replaces the "has_mtdata" with "is_supported" logic, and lets some non-MT devices
through. That should take care of the risk of regression for other devices.

[Chase Douglas (chasedouglas)]

It sounds like we're going to try to push this into maverick. I'm going to do some testing on it to be sure it doesn't kill my cat :).

[Duncan McGreggor (oubiwann)]

Yeah, we try to support animal-friendly products. Thanks for keeping that in mind, Chase.

[Chase Douglas (chasedouglas)]

Well, it didn't kill my cat, but it did kill my single touch cursor motion.

In lines 148, 153, and 158 in the big diff below, the return value should be 0 instead of 1 (according to Henrik). This fixed my issues.

review: Needs Fixing

[Henrik Rydberg (rydberg)]

Pacthes for two problems have been folded into the originals: 1) set minor when not available from the device, and 2) let grail-touch be transparent when driving a non-mt device. The latter caused the pointer position in the legacy emulation in grail-gesture to not be updated.

[Chase Douglas (chasedouglas)]

The synaptics functionality seems to work right for me now. However, I noticed another issue: four finger pinches are often recognized as taps. I believe the issue lies in the fact that the motion of the focus point inhibits a tap, but a pinch may not move the focus point. I have a proposed patch to fix this, and I'd like to see a fix end up in 1.0.13.

review: Needs Fixing

[Henrik Rydberg (rydberg)]

The triggers are on difference between absolute numbers, whereas here we want to add change, and trigger on the value of that change, which is not consistent (ie wont work properly).

review: Needs Fixing

[Henrik Rydberg (rydberg)]

repack3, adding two more patches, thanks to Chase for finding those bugs.

[Henrik Rydberg (rydberg)]

repack4, hopefully the final one..

=== modified file 'configure.ac'
--- configure.ac	2010-09-01 21:01:08 +0000
+++ configure.ac	2010-09-10 12:20:58 +0000
@@ -1,7 +1,7 @@
 # Initialize Autoconf
 AC_PREREQ([2.60])
 AC_INIT([Gesture Recognition And Instantiation Library],
-        [1.0.12],
+        [1.0.13],
         [],
         [utouch-grail])
 AC_CONFIG_SRCDIR([Makefile.am])

=== modified file 'include/grail-touch.h'
--- include/grail-touch.h	2010-08-19 20:26:11 +0000
+++ include/grail-touch.h	2010-09-10 12:20:58 +0000
@@ -72,6 +72,7 @@
 	int state;
 };
 
+int touch_caps_is_supported(struct touch_dev *dev, int fd);
 void touch_caps_init(struct touch_dev *dev);
 float touch_angle(const struct touch_dev *dev, int orientation);
 float touch_pressure(const struct touch_dev *dev, int pressure);

=== modified file 'src/gestures-tapping.c'
--- src/gestures-tapping.c	2010-08-26 14:41:20 +0000
+++ src/gestures-tapping.c	2010-09-10 12:20:58 +0000
@@ -24,8 +24,9 @@
 #include <math.h>
 
 static const int TAP_MIN_GAP_MS = 25;
+static const int TAP_MIN_UP_MS = 200;
 
-static const int fm_mask = 0x03;
+static const int fm_mask = 0x07;
 
 static void set_props(const struct gesture_inserter *gin,
 		      struct tapping_model *s, const struct move_model *m,
@@ -44,6 +45,9 @@
 	struct gesture_recognizer *gru = ge->gru;
 	struct tapping_model *state = &gru->tapping;
 	struct move_model *move = &gru->move;
+	grail_time_t up = move->time - state->end;
+	if (move->ntouch && move->ntouch < state->ntouch)
+		state->end = move->time;
 	if (move->ntouch > state->ntouch) {
 		if (state->active) {
 			gin_gid_discard(ge, state->gid);
@@ -67,7 +71,8 @@
 		int x = state->prop[GRAIL_PROP_TAP_X];
 		int y = state->prop[GRAIL_PROP_TAP_Y];
 		int t = move->time - state->start;
-		if (t < TAP_MIN_GAP_MS) {
+		if (t < TAP_MIN_GAP_MS || t > move->fm[FM_X].bar_ms ||
+		    up < TAP_MIN_UP_MS) {
 			gin_gid_discard(ge, state->gid);
 			state->ntouch = move->ntouch;
 			state->active = 0;

=== modified file 'src/grail-api.c'
--- src/grail-api.c	2010-08-26 14:40:35 +0000
+++ src/grail-api.c	2010-09-10 12:20:58 +0000
@@ -92,20 +92,27 @@
 	return nevent;
 }
 
+static int pointer_active(const struct grail *ge)
+{
+	static const int fm_mask = 0x03;
+	const struct move_model *move = &ge->gru->move;
+	const struct tapping_model *tap = &ge->gru->tapping;
+	return move->single && ((move->active & fm_mask) || tap->active);
+}
+
 static void tp_sync(struct touch_dev *dev,
 		    const struct input_event *syn)
 {
 	struct input_event ev;
 	struct grail *ge = dev->priv;
 	struct grail_impl *impl = ge->impl;
-	struct gesture_recognizer *gru = ge->gru;
 	struct touch_frame *frame = &dev->frame;
 	grail_mask_t filtered[DIM_EV_TYPE_BYTES];
 	int pointer, nevent = 0;
 	gin_frame_begin(ge, frame);
 	gru_recognize(ge, frame);
 	gin_frame_end(ge, filtered, sizeof(filtered), frame);
-	pointer = gru->move.single && !grail_mask_get(filtered, EV_ABS);
+	pointer = pointer_active(ge) && !grail_mask_get(filtered, EV_ABS);
 
 	if (!ge->event) {
 		evbuf_clear(&impl->evbuf);

=== modified file 'src/grail-gestures.c'
--- src/grail-gestures.c	2010-09-01 21:00:37 +0000
+++ src/grail-gestures.c	2010-09-10 12:20:58 +0000
@@ -26,7 +26,7 @@
 
 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] = { 200, 200, 10000, 10000 };
+static const grail_time_t FM_BAR_MS[DIM_FM] = { 300, 300, 10000, 10000 };
 static const grail_time_t SAMPLE_MS = 10;
 static const float EPS = 1e-3;
 

=== modified file 'src/grail-gestures.h'
--- src/grail-gestures.h	2010-08-26 14:41:20 +0000
+++ src/grail-gestures.h	2010-09-10 12:20:58 +0000
@@ -97,7 +97,7 @@
 		  const struct touch_frame *frame);
 
 struct tapping_model {
-	grail_time_t start;
+	grail_time_t start, end;
 	int ntouch;
 	int active, gid;
 	int nprop;

=== modified file 'src/touch-caps.c'
--- src/touch-caps.c	2010-08-31 12:20:32 +0000
+++ src/touch-caps.c	2010-09-10 12:20:58 +0000
@@ -21,6 +21,7 @@
  ****************************************************************************/
 
 #include <grail-touch.h>
+#include <errno.h>
 #include <math.h>
 
 /* see mtdev-mapping.h */
@@ -29,6 +30,42 @@
 #define MTDEV_ORIENTATION	4
 #define MTDEV_PRESSURE		10
 
+#define SYSCALL(call) while (((call) == -1) && (errno == EINTR))
+
+static const int bits_per_long = 8 * sizeof(long);
+
+static inline int nlongs(int nbit)
+{
+	return (nbit + bits_per_long - 1) / bits_per_long;
+}
+
+static inline int getbit(const unsigned long *map, int key)
+{
+	return (map[key / bits_per_long] >> (key % bits_per_long)) & 0x01;
+}
+
+int touch_caps_is_supported(struct touch_dev *dev, int fd)
+{
+	unsigned long keybits[nlongs(KEY_MAX)];
+	unsigned long absbits[nlongs(ABS_MAX)];
+	int rc;
+
+	if (dev->mtdev.caps.has_mtdata)
+		return 1;
+
+	SYSCALL(rc = ioctl(fd, EVIOCGBIT(EV_KEY, sizeof(keybits)), keybits));
+	if (rc < 0)
+		return 0;
+	SYSCALL(rc = ioctl(fd, EVIOCGBIT(EV_ABS, sizeof(absbits)), absbits));
+	if (rc < 0)
+		return 0;
+
+	return getbit(absbits, ABS_X) &&
+		getbit(absbits, ABS_Y) &&
+		getbit(keybits, BTN_TOUCH) &&
+		getbit(keybits, BTN_TOOL_DOUBLETAP);
+}
+
 void touch_caps_init(struct touch_dev *dev)
 {
 	const struct mtdev_caps *mt = &dev->mtdev.caps;

=== modified file 'src/touch-dev.c'
--- src/touch-dev.c	2010-08-19 20:26:11 +0000
+++ src/touch-dev.c	2010-09-10 12:20:58 +0000
@@ -25,6 +25,9 @@
 #include <string.h>
 #include <errno.h>
 
+/* see mtdev-mapping.h */
+#define MTDEV_TOUCH_MINOR	1
+
 #define SET_PROP(name, value)						\
 	if (t->name != value) {						\
 		t->name = value;					\
@@ -34,6 +37,9 @@
 static void finish_touch(struct touch_dev *dev, struct touch_frame *frame)
 {
 	struct touch *t = &frame->touch[dev->slot];
+	if (t->touch_major && !t->touch_minor ||
+	    !dev->mtdev.caps.has_abs[MTDEV_TOUCH_MINOR])
+		SET_PROP(touch_minor, t->touch_major);
 	if (dev->state > 0) {
 		t->active = 1;
 		grail_mask_set(frame->touches, dev->slot);
@@ -47,13 +53,45 @@
 	dev->state = 0;
 }
 
+static void finish_legacy(struct touch_dev *dev, struct touch_frame *frame)
+{
+	static int trkid;
+	int i;
+	for (i = 0; i < frame->nactive; i++) {
+		struct touch *t = &frame->touch[i];
+		t->active = 1;
+		if (t->id < 0) {
+			t->id = trkid++ & 0xffff;
+			frame->ncreate++;
+		}
+		if (i > 0) {
+			SET_PROP(x, frame->touch[0].x);
+			SET_PROP(y, frame->touch[0].y);
+			SET_PROP(pressure, frame->touch[0].pressure);
+		}
+		grail_mask_set(frame->touches, i);
+	}
+	for (i = frame->nactive; i < DIM_TOUCH; i++) {
+		struct touch *t = &frame->touch[i];
+		t->active = 0;
+		if (t->id >= 0) {
+			t->id = -1;
+			frame->ndestroy++;
+		}
+		grail_mask_clear(frame->touches, i);
+	}
+}
+
 static void finish_packet(struct touch_dev *dev,
 			  const struct input_event *syn)
 {
 	static const touch_time_t ms = 1000;
 	struct touch_frame *frame = &dev->frame;
 	int i, nslot = 0;
-	finish_touch(dev, frame);
+	if (dev->mtdev.caps.has_mtdata)
+		finish_touch(dev, frame);
+	else
+		finish_legacy(dev, frame);
 	grail_mask_foreach(i, frame->touches, DIM_TOUCH_BYTES)
 		frame->active[nslot++] = &frame->touch[i];
 	frame->nactive = nslot;
@@ -71,6 +109,21 @@
 	struct touch_frame *frame = &dev->frame;
 	struct touch *t = &frame->touch[dev->slot];
 	switch (ev->code) {
+	case ABS_X:
+		if (dev->mtdev.caps.has_mtdata)
+			return 0;
+		SET_PROP(x, ev->value);
+		return 0;
+	case ABS_Y:
+		if (dev->mtdev.caps.has_mtdata)
+			return 0;
+		SET_PROP(y, ev->value);
+		return 0;
+	case ABS_PRESSURE:
+		if (dev->mtdev.caps.has_mtdata)
+			return 0;
+		SET_PROP(pressure, ev->value);
+		return 0;
 	case ABS_MT_SLOT:
 		if (ev->value >= 0 && ev->value < DIM_TOUCH) {
 			if (dev->slot != ev->value)
@@ -122,6 +175,40 @@
 	}
 }
 
+static int handle_key_event(struct touch_dev *dev,
+			    const struct input_event *ev)
+{
+	struct touch_frame *frame = &dev->frame;
+	if (dev->mtdev.caps.has_mtdata)
+		return 0;
+	switch (ev->code) {
+	case BTN_TOUCH:
+		if (ev->value && !frame->nactive)
+		    frame->nactive = 1;
+		else if (!ev->value && frame->nactive)
+		    frame->nactive = 0;
+		return 0;
+	case BTN_TOOL_FINGER:
+		if (ev->value)
+			frame->nactive = 1;
+		return 0;
+	case BTN_TOOL_DOUBLETAP:
+		if (ev->value)
+			frame->nactive = 2;
+		return 0;
+	case BTN_TOOL_TRIPLETAP:
+		if (ev->value)
+			frame->nactive = 3;
+		return 0;
+	case BTN_TOOL_QUADTAP:
+		if (ev->value)
+			frame->nactive = 4;
+		return 0;
+	default:
+		return 0;
+	}
+}
+
 int touch_dev_open(struct touch_dev *dev, int fd)
 {
 	struct touch_frame *frame = &dev->frame;
@@ -133,7 +220,7 @@
 		t->id = MT_ID_NULL;
 	}
 	ret = mtdev_open(&dev->mtdev, fd);
-	if (!ret && !dev->mtdev.caps.has_mtdata)
+	if (!ret && !touch_caps_is_supported(dev, fd))
 		ret = -ENODEV;
 	if (ret)
 		goto error;
@@ -160,6 +247,8 @@
 			consumed++;
 		} else if (ev.type == EV_ABS) {
 			consumed += handle_abs_event(dev, &ev);
+		} else if (ev.type == EV_KEY) {
+			consumed += handle_key_event(dev, &ev);
 		}
 		if (!consumed && dev->event)
 			dev->event(dev, &ev);