GNOME Bugzilla – Bug 685028
Let users and subclasses set a custom threshold on GestureAction
Last modified: 2012-10-19 09:32:51 UTC
The usefulness of having an overridable :threshold property in GestureAction would be twofold: • users like examples/pan-action may set it to zero to be more responsive; • an hypothetical Tap gesture may override the property and use it as the max distance before cancelling the gesture, while setting the original GestureAction property to zero. The patch attached adds the ClutterGestureAction:threshold property and falls back to ClutterSettings:dnd-drag-threshold if set to -1. For convenience I've assumed that there will be a 1.14 release and I've added the needed macros. :)
Created attachment 225318 [details] [review] version: Add 1.14 version macros
Created attachment 225319 [details] [review] Add the GestureAction:threshold property Let gesture users or subclasses set a custom threshold, falling back to :dnd-drag-threshold otherwise: • examples/pan-action sets it to zero to be more responsive; • an hypothetical Tap gesture may override the property and use it as the max distance before cancelling the gesture, while setting the original GestureAction property to zero.
Created attachment 225497 [details] [review] Add the GestureAction:threshold property Fix some issues encountered while developing ClutterTapAction on top of this patch.
Created attachment 225501 [details] [review] Add the GestureAction:threshold property Honour the :threshold property while checking if a gesture should be started when invoking clutter_gesture_action_set_n_touch_points().
Created attachment 225869 [details] [review] Add the GestureAction:threshold property Removed an unused variable and fixed the threshold condition.
Created attachment 226119 [details] [review] version: Add 1.14 version macros Oops, I forgot the CLUTTER_VERSION_1_14 macro.
Review of attachment 226119 [details] [review]: LGTM
Review of attachment 225869 [details] [review]: ::: clutter/clutter-gesture-action.c @@ +227,3 @@ + gint threshold) +{ + g_return_if_fail (threshold >= -1); do not use g_return_if_fail() inside internal/static functions. use g_assert() if you want to verify internal state. @@ +244,3 @@ + ClutterSettings *settings = clutter_settings_get_default (); + g_object_get (settings, + "dnd-drag-threshold", &threshold, coding style: braces on a separate line, and argument alignment of g_object_get() @@ +286,3 @@ + + g_signal_emit (action, gesture_signals[GESTURE_BEGIN], 0, actor, + &return_value); coding style: argument alignment @@ +347,1 @@ + if ((ABS (point->press_y - motion_y) < drag_threshold) && you may want to use fabsf() instead of the ABS macro. @@ +1053,3 @@ + g_return_if_fail (CLUTTER_IS_GESTURE_ACTION (action)); + + g_object_set (G_OBJECT (action), "threshold", threshold, NULL); eeeewwwwww, no, no, a thousand times *no*. if you want a way to override the *meaning* of a threshold then I think we should just rip out the whole concept of threshold in GestureAction altogether, and define ad hoc gesture thresholds in its subclasses. alternatively, use negative numbers for "cancel the gesture if the threshold is passed" and positive numbers for "begin the gesture if the threshold is passed". but I'd rather have the meaning of threshold be a per-GestureAction class decision.
Comment on attachment 226119 [details] [review] version: Add 1.14 version macros Attachment 226119 [details] pushed as cb4620d - version: Add 1.14 version macros
(In reply to comment #8) > + g_return_if_fail (CLUTTER_IS_GESTURE_ACTION (action)); > + > + g_object_set (G_OBJECT (action), "threshold", threshold, NULL); > > eeeewwwwww, no, no, a thousand times *no*. > > if you want a way to override the *meaning* of a threshold then I think we > should just rip out the whole concept of threshold in GestureAction altogether, > and define ad hoc gesture thresholds in its subclasses. > > alternatively, use negative numbers for "cancel the gesture if the threshold is > passed" and positive numbers for "begin the gesture if the threshold is > passed". but I'd rather have the meaning of threshold be a per-GestureAction > class decision. I don't see it as a big problem (it's still a threshold, I just want to override the direction of the threshold) but if you prefer I can try some other way. What about splitting it in ::begin-threshold (defaults to -1 → ::dnd-drag-threshold) and ::cancel-threshold (defaults to -1 → no threshold)? Do you have other suggestions?
Created attachment 226526 [details] [review] gesture-action: Let subclasses override the GestureTriggerEdge handling Let gesture subclasses override how the drag threshold should be handled: • CLUTTER_GESTURE_TRIGGER_NONE tells GestureAction that the gesture must begin immediately and there's no drag limit that will cause its cancellation; • CLUTTER_GESTURE_TRIGGER_AFTER is the default GestureAction behaviour, where it needs to wait until the drag threshold has been exceeded before considering the gesture valid; • CLUTTER_GESTURE_TRIGGER_BEFORE will make GestureAction cancel the gesture once the drag exceed the configured threshold. For example, ZoomAction and RotateAction could set CLUTTER_GESTURE_TRIGGER_NONE since the use of two fingers makes the begin of the action more self-evident, while an hypothetical Tap gesture may use CLUTTER_GESTURE_TRIGGER_BEFORE to cancel the tap if the pointer moves too much.
Review of attachment 226526 [details] [review]: looks okay to me
The following fix has been pushed: abcf1d5 gesture-action: Let subclasses override the GestureTriggerEdge handling
Created attachment 226803 [details] [review] gesture-action: Let subclasses override the GestureTriggerEdge handling Let gesture subclasses override how the drag threshold should be handled: • CLUTTER_GESTURE_TRIGGER_NONE tells GestureAction that the gesture must begin immediately and there's no drag limit that will cause its cancellation; • CLUTTER_GESTURE_TRIGGER_AFTER is the default GestureAction behaviour, where it needs to wait until the drag threshold has been exceeded before considering the gesture valid; • CLUTTER_GESTURE_TRIGGER_BEFORE will make GestureAction cancel the gesture once the drag exceed the configured threshold. For example, ZoomAction and RotateAction could set CLUTTER_GESTURE_TRIGGER_NONE since the use of two fingers makes the begin of the action more self-evident, while an hypothetical Tap gesture may use CLUTTER_GESTURE_TRIGGER_BEFORE to cancel the tap if the pointer moves too much.