GNOME Bugzilla – Bug 678586
Modifiy ClutterGestureAction to support multi touch and multiple points
Last modified: 2012-07-17 00:31:33 UTC
Currently the ClutterGestureAction only supports 1 pointer input. The attached patch add support for multiple points as well as touch events.
Created attachment 216985 [details] [review] Patch v1 This patch contains a couple of lines of code in comment. I'm not really sure whether this commented check is useful for something else... but it prevents stuff like DragAction/ZoomAction/RotateAction to be implemented on top of the GestureAction.
Review of attachment 216985 [details] [review]: generally, yes: this is what I envisioned for GestureAction. ::: clutter/clutter-gesture-action.c @@ +63,3 @@ #include "clutter-private.h" +#define MAX_GESTURE_POINTS (10) the size is small, but I don't entirely like to preallocate 320 bytes per gesture (on 32 bit; on 64 is going to be 400 bytes) on top of the contents of the GestureAction and its subclasses. we could use a GArray instead, and prealloc 2 or 3 slots, considering that the majority of the gestures are going to involve a two or three touch points. @@ +239,1 @@ + /* if (!clutter_actor_transform_stage_point (actor, */ are you using GestureAction with direct children of the Stage? what happens when you start packing actors with gestures inside containers? also: are you using a touchscreen device? the coordinate space changes between direct touch devices and indirect touch devices; I think this will need some API inside ClutterInputDevice to be able to distinguish the cases. the documentation says that the coordinates we pass to the signal handlers are always actor-relative, not stage-relative, so we need the transformation here. @@ +614,3 @@ + * @action: a #ClutterGestureAction + * + * Retrieves the number of requested points to trigger the gesture. "the number of touch points required to begin the gesture." @@ +633,3 @@ + * @nb_points: a number of points + * + */ same as above. ::: clutter/clutter-gesture-action.h @@ +103,3 @@ ClutterAction * clutter_gesture_action_new (void); +gint clutter_gesture_action_get_n_touch_points (ClutterGestureAction *action); missing CLUTTER_AVAILABLE_IN_1_12 annotations
Review of attachment 216985 [details] [review]: ::: clutter/clutter-gesture-action.c @@ +239,1 @@ + /* if (!clutter_actor_transform_stage_point (actor, */ First most of the gestures do not really care about relative coordinates, they mostly operate on vectors. Then the way the function was called shows that the coordinates where never transformed before. And finally during my tests this function often prevented the emission of the GESTURE_UPDATE signal, I need to check why the transformation might fail.
the only two reasons I know of that the transformation may fail are: - the actor has either size of 0; - the actor's has been transformed into a line through rotation or scaling. and you're right, the transformation is a check on whether we can actually transform the coordinates in actor space: I was confused by the code in DragAction, which does transform the event coordinates into actor space because of the clutter_actor_move_by() call inside drag-motion, as well as passing the delta inside the signal handler. I think the transformation requirement should be dropped; Tomeu wrote GestureAction, so he may have an idea of whether it's something that was meant to catch some issue or not.
Created attachment 217330 [details] [review] patch v2
Review of attachment 217330 [details] [review]: looks good, apart from a couple of minor details. ::: clutter/clutter-gesture-action.c @@ +330,3 @@ return CLUTTER_EVENT_PROPAGATE; + point = gesture_register_point (action, event); this should warn if point == NULL - though you don't use point later on anyway. ::: clutter/clutter-gesture-action.h @@ +103,3 @@ ClutterAction * clutter_gesture_action_new (void); +gint clutter_gesture_action_get_n_touch_points (ClutterGestureAction *action); missing CLUTTER_AVAILABLE_IN_1_12 annotation. @@ +104,3 @@ +gint clutter_gesture_action_get_n_touch_points (ClutterGestureAction *action); +void clutter_gesture_action_set_n_touch_points (ClutterGestureAction *action, missing CLUTTER_AVAILABLE_IN_1_12 annotation.
Created attachment 218652 [details] [review] patch v3 Fixed the points in the review
Review of attachment 218652 [details] [review]: looks good.
landed.