Merge lp:~tkluck/grail/coordinate-transform into lp:grail

[Timo Kluck]

See https://bugs.launchpad.net/utouch-grail/+bug/788128.
Depends on https://code.launchpad.net/~tkluck/utouch-frame/coordinate-transform/+merge/73390

[Chase Douglas (chasedouglas)]

Small note: I don't think the callback needs to be in a new deprecated api section. You can add it just below grail_get_contact_frame() in grail.h.

The changes use malloc to allocate memory. malloc is not signal context safe (see "safe functions" in the signal(2) manpage). You probably aren't aware, or might have forgotten if you were (I almost forgot :), that X input module event handling is run in signal context. If we were to merge this code, we would find it works just fine on many machines, usually fine but occasionally crashy on some machines, and very crashy on other machines. It's a very indeterminate bug, and very difficult to try to figure out!

I'm not sure how to get around this. You could try alloca, but it's not an officially "safe" function to call in signal context, and in general alloca use is frowned upon. You could try to compile it with c99 standard variable length arrays, but I don't know if that's safe either, and we would have to switch grail to a c99 library. Alternatively, you could allocate a constant number of touches, but there's no bounds on how many touches there might actually be. If you set it to 10 touches and then ran it on a 20 touch monitor and touched with 11 touches you would crash.

This all begins to feel like this isn't the best place to implement the transformation. Perhaps the transformation should be called from within utouch-frame instead.

I really appreciate the thought and effort you put into this! I don't want you to be discouraged by my disapproval, especially since it is likely due to something you weren't aware of. I look forward to reviewing another iteration of changes :).

review: Disapprove

[lp:~tkluck/grail/coordinate-transform]

169. By Timo Kluck on 2011-08-30

moved coordinate transformation to utouch-frame

170. By Timo Kluck on 2011-08-31

Bump version (note API extension)

Unmerged revisions

=== modified file 'include/grail.h'
--- include/grail.h	2011-05-18 16:41:41 +0000
+++ include/grail.h	2011-08-30 15:14:00 +0000
@@ -468,6 +468,11 @@
 
 #endif
 
+void GRAIL_PUBLIC grail_set_coordinate_transform_callback(struct grail *ge,
+							  utouch_coordinate_transform_cb callback,
+							  void *user_data);
+
+
 #ifdef __cplusplus
 }
 #endif

=== modified file 'src/grail-api.c'
--- src/grail-api.c	2011-08-09 10:03:32 +0000
+++ src/grail-api.c	2011-08-30 15:14:00 +0000
@@ -321,3 +321,9 @@
 
         return count > 0 ? count : ret;
 }
+
+void GRAIL_PUBLIC grail_set_coordinate_transform_callback(struct grail *ge,
+							  utouch_coordinate_transform_cb callback,
+							  void *user_data) {
+	utouch_frame_set_coordinate_transform_callback(ge->impl->fh, callback, user_data);
+}