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.

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

79. By Henrik Rydberg on 2010-09-10

Make sure contact area is always defined

Some devices do not always send touch_minor after touch_major
has been set. According to the MT protocol, this case should
be treated as if minor is equal to major. This patch makes sure
touch_minor is never zero when touch_major is set.

80. By Henrik Rydberg on 2010-09-10

Support legacy devices

Simulate MT touch events for legacy devices that support the
DOUBLETAP, TRIPLETAP and QUADTAP events. This allows multi-finger
tap and drag gestures to be emitted, providing a basic level of
MT functionality for legacy devices.

81. By Henrik Rydberg on 2010-09-10

Discard tapping even when there are zero events between dow
n and up

The current logic assumes there will be at least one frame with
between touch up and touch down or the tap will not be properly
timed out. Fixed with this patch.

Reported-by: Chase Douglas <email address hidden>

82. By Henrik Rydberg on 2010-09-10

Cancel tapping also on zoom events

The current code cancels tapping when dragging is active. This
patch adds zooming to the set, to inhibit tapping also when a
perfect zoom without dragging is performed.

Reported-by: Chase Douglas <email address hidden>

83. By Henrik Rydberg on 2010-09-10

Only emit abs events when motion is active

This bug was hidden behind the motion inhibit during tapping,
but resurfaced when tapping from resting finger was introduced.
With this patch, pointer movement is inhibited also when lifting
fingers from the surface.

Ideally, tapping should be treated as a transient event, which would
allow zero pointer event also _before_ the tap, but presently unity
seems to depend on the initial pointer movement as a touch-down event.

84. By Henrik Rydberg on 2010-09-10

Inhibit taps immediately after a finger is lifted

The intentional tap starts with a finger in the air. This patch
treats the sequence up-down-up as unintentional, unless performed
with a single finger. Fixes the problem with accidental taps during
multi-finger gestures.

85. By Henrik Rydberg on 2010-09-10

Allow slow tapping

For the benefit of hardware that cannot accurately meet the desirable
response times, allow for tapping times up to 300 ms.

86. By Henrik Rydberg on 2010-09-10

grail v1.0.13

Unmerged revisions

=== modified file 'configure.ac'
--- configure.ac	2010-09-01 21:01:08 +0000
+++ configure.ac	2010-09-10 12:15:55 +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:15:55 +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:15:55 +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:15:55 +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:15:55 +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:15:55 +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:15:55 +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:15:55 +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);