Page MenuHomePhabricator

Create Gesture Framework in Elementary
Open, HighPublic

Description

Elm_Gesture_Layer should disapear and just become event you register on the window (maybe on every widget ?) that start to track your mouse action when you listen on them (adding logic to watch callback add/del on the window widget).

cedric created this task.Mar 30 2017, 2:43 AM
smohanty claimed this task.Jun 26 2017, 7:15 PM

Maybe let it be a custom object, that you can add with compose to a object where you want the events to be emitted ? So you could for example only have gestures on one speicific widget. (And we have a seperated logical unit that does the gesture stuff)

After going through the current implementation and referring to the other framework how they implement gesture i came up with the following design.

  1. GestureEvent Base Structure

  1. All the api related to a custom gesture(swap, zoom , tap) should move to its own class/structure

  1. The Gesture Recognizer part should be moved to separate class so that its easier to maintain and add new recognizer

  1. The GestureManager Should be per instance of a window and should handle the gesture request from the widget and delegating to the gesture recognizer

From User perspective it will just look for the Gesture Event as any other event
evas_object_event_callback_add(rect, EVAS_CALLBACK_TAP_GESTURE, _tap_gesture_cb, NULL);
_tap_gesture_cb(Void *data, Gesture_Event *ev)
{

 //1.  get the tap gesture structure from the event.
// 2. check for the gesture state and do appropriate action.

}

Current Implementation of Gesture in Elm for your reference

  1. Elm_Gesture_Layer manages both recognizer as well as the handling of callback.
  2. User has to add different callback for different state of the same gesture(like tap gesture start, stop, finish, abort ) user has to add 4 callback instead of one.
  3. every object who needs to get the gesture has to create a additional object (gesture_layer object)

Please let me know your feedback

I also think gesture does not need to be supported with some special ways which are different from the common event supporting way.
(i.e. I don't like current way with elm_gesture_layer)
But, don't know well about if there can be any TECHNICAL problem.

@cedric @jpeg
Could you give any idea on @smohanty comment ?

Additionally, What could be the benefits from this change ?
If there is no big benefit from this work, we need to think about setting lower prioity on this work.

jpeg added a comment.Jul 10 2017, 3:22 AM

From User perspective it will just look for the Gesture Event as any other event
evas_object_event_callback_add(rect, EVAS_CALLBACK_TAP_GESTURE, _tap_gesture_cb, NULL);

Obviously I assume efl_event here, not evas. Thus not an enum but a random event name (more flexible).

Current Implementation of Gesture in Elm for your reference

  1. Elm_Gesture_Layer manages both recognizer as well as the handling of callback.
  2. User has to add different callback for different state of the same gesture(like tap gesture start, stop, finish, abort ) user has to add 4 callback instead of one.

I agree 1 is probably a better idea. Although the idea was probably to only have a single "finish" callback, and not care about other states (eg. for a swipe or double tap).

  1. every object who needs to get the gesture has to create a additional object (gesture_layer object)

Ugly... :)

Considering this is a simple API, which I'm happy about, I think we still need to make it future-proof:

  • We may need some way for users to insert their own gesture recognisers as well. This also means registering new events (eg. MY_APP_EVENT_TWO_FINGER_SWIPE).
  • As Cedric said, in terms of API this should be a single event callback add/del in order to automatically register an event recogniser and all that.
In T5320#90947, @jpeg wrote:

From User perspective it will just look for the Gesture Event as any other event
evas_object_event_callback_add(rect, EVAS_CALLBACK_TAP_GESTURE, _tap_gesture_cb, NULL);

Obviously I assume efl_event here, not evas. Thus not an enum but a random event name (more flexible).

Seems like someone hasn't looked at Efl_Event :-) Please look at the use of EFL_EVENT_CALLBACK_ADD and EFL_EVENT_CALLBACK_DEL. There are example in src/lib/evas/canvas/evas_callbacks.c .

Current Implementation of Gesture in Elm for your reference

  1. Elm_Gesture_Layer manages both recognizer as well as the handling of callback.
  2. User has to add different callback for different state of the same gesture(like tap gesture start, stop, finish, abort ) user has to add 4 callback instead of one.

I agree 1 is probably a better idea. Although the idea was probably to only have a single "finish" callback, and not care about other states (eg. for a swipe or double tap).

I like the overall design with a manager too and if it can get the ability to add custom gesture recognizer, it would be pretty good and future proof indeed.

cedric raised the priority of this task from TODO to High.Jul 10 2017, 3:03 PM
smohanty renamed this task from Gesture should be event on the window to Create Gesture Framework in Elementary.Jul 23 2017, 9:25 PM

I have added a private branch dev/subhransu/gesture for the above task. And pushed a commit containing the eo files required for the gesture framework

jpeg added a comment.Jul 23 2017, 10:33 PM

I have added a private branch dev/subhransu/gesture for the above task. And pushed a commit containing the eo files required for the gesture framework

I had a quick look, here are some comments:

  • Don't rely on an enum for the gesture types. This can't work with custom gestures. I think the event name is enough, as EO events are basically unique runtime identifiers.
  • The methods recognize and create in the Recognizer are a bit strange. I imagine only one function "feed" similar to your "recognize" should be enough. The feed method shouldn't take any gesture as input. The recognizer can create the gestures info itself. Reset is fine.
  • unregister_recognizer should take in the same argument as the register function, i.e. a recognizer :)

The rest is fine.

Hi All, @cedric @jpeg @raster
I have pushed the initial implementation of gesture framework to my private branch dev/subhransu/gesture
could you please review and let me know your feedback.

Thanks
Subhransu

@cedric , @jpeg

I have incorporated the review comment provided by JP.

Here are the few points that yet to be resolved.

  1. Should we process the gesture event before or after the widget gets normal mouse event ? (currently gesture manager processes the event first)
  2. How to implement the double tap and triple tap gesture with the current gesture framework ? (As with the current implementation recognizer uses the event from mouse down to mouse up to decide the gesture but in double tap case it has to alive for 2 consecutive touch sequence)

@jpeg , Woohyun has the opinion to put this commit to master and then request for the feedback as it is still in experimental stage. what do you think of it ?

Thanks
Subhransu

jpeg added a comment.Oct 15 2017, 11:12 PM

@cedric , @jpeg

I have incorporated the review comment provided by JP.

Here are the few points that yet to be resolved.

  1. Should we process the gesture event before or after the widget gets normal mouse event ? (currently gesture manager processes the event first)

@cedric any idea on this?

  1. How to implement the double tap and triple tap gesture with the current gesture framework ? (As with the current implementation recognizer uses the event from mouse down to mouse up to decide the gesture but in double tap case it has to alive for 2 consecutive touch sequence)

Ideally elm_gesture_layer should be using this gesture framework. If it can't be done we need to know exactly why not.

@jpeg , Woohyun has the opinion to put this commit to master and then request for the feedback as it is still in experimental stage. what do you think of it ?

I see that some of my comments still haven't been addressed, especially for Efl.Gesture.Touch (out values, etc...)
I'd like to have a solid, clean looking API, before merging it to master. The internals aren't as important as getting the API right.

In T5320#101757, @jpeg wrote:

Here are the few points that yet to be resolved.

  1. Should we process the gesture event before or after the widget gets normal mouse event ? (currently gesture manager processes the event first)

@cedric any idea on this?

There is a big difference with the way we are able to do thing with the hook logic here. We would not intercept input if there is nobody registered for that event, so it seems to me it is safe to actually process the event first. But I think it require real tests of this and see if it doesn't create problem.

@smohanty
Would you share the plan for this task ?

Do we still have plan for this release regarding this ?

@woohyun , @cedric
We still have some open points
2.How to implement the double tap and triple tap gesture with the current gesture framework ? (As with the current implementation recognizer uses the event from mouse down to mouse up to decide the gesture but in double tap case it has to alive for 2 consecutive touch sequence).

here are the task that are still pending.

  1. Formal review of the API (the codebase is already merged to the master ) so we need to finalize if the api is enough to make it public.
  2. Add more gesture implementation(swipe , pinch & zoom). we can incrementally add those gestures after the API of gesture framework finalized.

I am currently busy with the lottie implementation. so if we can decide the way further then i will prioritize this task accordingly.

raster added a comment.Apr 3 2018, 3:30 AM

i agree that this is something that can be expanded over time as long as there is a concept of "cancelling" and we agree that many actions (gestures) are higher level and may have to be "time deferred" until later, or "distance deferred". e.g. you have to delay a single click/tap gesture until you KNOW you can't possibly have a double tap (or a triple tap). a long press only begins after holding for time X without moving too far (how far is too far needs to be a tunable value - same with click, vs double click vs triple click vs long press etc.).

zmike edited projects, added Restricted Project; removed efl.Jun 11 2018, 6:54 AM
bu5hm4n edited projects, added efl: widgets; removed Restricted Project.Jun 11 2018, 9:15 AM
zmike added a subscriber: zmike.

A #Goal ticket should not be set to a milestone.

I think this is done ?