Page MenuHomePhabricator

Gesture Manager: Add gestures and fix gesture managing, recognizer logic.
ClosedPublic

Authored by CHAN on Jan 10 2019, 1:16 AM.

Details

Summary

https://phab.enlightenment.org/T7544

Provides a way for a user to get a gesture manager, recognizer instance.

Supports different recognizer properties for each target(Eo).

Gesture, Touch Class Life-cycle re-implementation. for supporting multiple touches.

Add below gestures.
efl_canvas_gesture_tap
efl_canvas_gesture_double_tap
efl_canvas_gesture_triple_tap
efl_canvas_gesture_long_tap
efl_canvas_gesture_momentum
efl_canvas_gesture_zoom
efl_canvas_gesture_flick

Test Plan

Simple test -> test_gesture_framework.c
More test cases will upload.

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
There are a very large number of changes, so older changes are hidden. Show Older Changes

I made several comments regarding the docs which have not been addressed. Please correct them.
I also suggest you mark each comment as "Done" so no comment is forgotten.

CHAN updated this revision to Diff 19900.Mar 3 2019, 11:29 PM

Add Zoom gesture.

CHAN updated this revision to Diff 19901.Mar 3 2019, 11:32 PM

Add zoom gesture to meson build.

CHAN updated this revision to Diff 19904.Mar 4 2019, 12:09 AM

Code clean up.

CHAN updated this revision to Diff 19905.Mar 4 2019, 12:09 AM

Code clean up.

CHAN added a comment.Mar 4 2019, 12:14 AM

I will upload whole doc.

Jaehyun_Cho added a comment.EditedMar 8 2019, 3:18 AM

@CHAN @smohanty

I have checked the code and tested the functionality.
And I found out that win cannot add gesture event callback.
Is it originally intended?

In my mind, it would be much easier and convenient if we add gesture event callback to win. Because we do not need to add rectangle only for gesture event callback.
Please let me know if I miss something or disadvantages of using win to add gesture event callback.

@CHAN @smohanty

Regardless of this patch, what do you think about changing method name "create" in Efl.Canvas.Gesture.Recognizer to "gesture_create"?

src/lib/evas/gesture/efl_canvas_gesture_recognizer.eo
16

Basically this is not related to this patch.
But what do you think about changing this method name to "gesture_add"?
If the method name remains "create", then language binding provides recognizer.Create() which does not look like creating gesture instance...

segfaultxavi requested changes to this revision.Mar 9 2019, 4:06 AM

Some of my comments regarding documentation have not been addressed yet...

src/lib/evas/gesture/efl_canvas_gesture_recognizer.eo
16

I agree Create will be confusing for the bindings.

This revision now requires changes to proceed.Mar 9 2019, 4:06 AM
Jaehyun_Cho added inline comments.Mar 10 2019, 7:40 PM
src/lib/evas/gesture/efl_canvas_gesture_recognizer.eo
16

I mean "gesture_create" not "gesture_add" :)

This is a minor suggestion of moving code inside the conditional expression.

src/lib/evas/gesture/efl_canvas_gesture_recognizer_double_tap.c
125

how about moving this inside the conditional expression if (pd->timeout) ?

139

how about moving this inside the conditional expression if (pd->timeout) ?

src/lib/evas/gesture/efl_canvas_gesture_recognizer_long_tap.c
119

how about moving this inside the conditional expression if (pd->timeout) ?

154

how about moving this inside the conditional expression if (pd->timeout) ?

src/lib/evas/gesture/efl_canvas_gesture_recognizer_tap.c
66

how about moving this inside the conditional expression if (pd->timeout) ?

src/lib/evas/gesture/efl_canvas_gesture_recognizer_triple_tap.c
125

how about moving this inside the conditional expression if (pd->timeout) ?

139

how about moving this inside the conditional expression if (pd->timeout) ?

This comment was removed by Jaehyun_Cho.
CHAN updated this revision to Diff 20452.Mar 11 2019, 11:37 PM
CHAN marked 12 inline comments as done.

Doc update and added code comment.

CHAN added a comment.Mar 11 2019, 11:42 PM

@segfaultxavi @Jaehyun_Cho

Thanks for giving review here.

I update code and doc.

I checked window target case.
It does not work well. The window has diff event generate logic.
I will be working independently from the current commit.

thanks.

src/lib/evas/gesture/efl_canvas_gesture_double_tap.eo
3

When user got gesture callback they need to use gesture methods for each gestures.

It defined a specific gesture event such as "tap" and user can get gesture infos.

More info about the gesture configure value will explain in recognizer class.

5

It generated to "EFL_EVENT_GESTURE_DOUBLE_TAP". does it wrong?

src/lib/evas/gesture/efl_canvas_gesture_long_tap.eo
3–4

It depends on the profile.

segfaultxavi resigned from this revision.Mar 12 2019, 2:39 AM

I have no further documentation concerns. I will let others do the code review.
Thanks!

The Structure looks ok as most of the changes are in the recognizer. this patch is good to go.

This revision is now accepted and ready to land.Mar 27 2019, 1:14 AM

This patch is going to be submitted after 1.22 release.

Jaehyun_Cho requested changes to this revision.Sun, Apr 28, 10:11 PM

@CHAN is updating this patch to support EFL C#.

This revision now requires changes to proceed.Sun, Apr 28, 10:11 PM
CHAN updated this revision to Diff 21760.Sun, Apr 28, 11:09 PM

Update code to supprt c# binding.

CHAN updated this revision to Diff 21762.Mon, Apr 29, 12:02 AM

Code clean up.

CHAN updated this revision to Diff 21764.EditedMon, Apr 29, 12:33 AM

Remove Conflict

CHAN updated this revision to Diff 21765.Mon, Apr 29, 12:40 AM

Fix build warning.

Could you check and remove EINA_UNUSED which are not supposed to be used?

src/lib/evas/gesture/efl_canvas_gesture_recognizer_double_tap.c
47

Please remove EINA_UNUSED

src/lib/evas/gesture/efl_canvas_gesture_recognizer_tap.c
29–30

I am not sure why but if Eo *obj EINA_UNUSED, then efl_data_scope_get(obj) returns garbage value.

Please remove EINA_UNUSED

33

Please remove EINA_UNUSED

src/lib/evas/gesture/efl_canvas_gesture_recognizer_triple_tap.c
47

Please remove EINA_UNUSED

CHAN updated this revision to Diff 21889.Thu, May 2, 5:19 AM
CHAN marked 2 inline comments as done.

Code clean up to following comments.

Jaehyun_Cho accepted this revision.Thu, May 2, 5:33 AM
This revision is now accepted and ready to land.Thu, May 2, 5:33 AM
Jaehyun_Cho requested changes to this revision.Thu, May 2, 5:37 AM

need to check meson build

This revision now requires changes to proceed.Thu, May 2, 5:37 AM

@CHAN

When I tried to build with meson, build error happens because efl_gesture_events.eo is dependent on efl_canvas_gesture_xxx.eo

In cxx meson build, eo files in efl/interfaces/ are only used for cxx bindings.

Because of the cxx meson build error, I am doubting if it is correct or not to make efl interface dependent on evas gesture classes..

CHAN updated this revision to Diff 22048.Wed, May 8, 11:45 PM

Meson build support

CHAN updated this revision to Diff 22074.Thu, May 9, 10:18 PM

mesong build support

Jaehyun_Cho accepted this revision.Tue, May 14, 12:36 AM
This revision is now accepted and ready to land.Tue, May 14, 12:36 AM
This revision was automatically updated to reflect the committed changes.

@Jaehyun_Cho Can you *please* *please* *please* be more carefull reviewing these meson changes. The eo files here are just plainly not installed anymore. because line 26 was removed. Additionally, its pure luck that this still works in meson, because you actually pass now a array with a array in array element nr. 0 followed by strings. This is not supported by meson and really only works by luck.

@bu5hm4n

I apologize. I've submitted a patch to fix it. 099eb2c315a277bda10f41709d7f936b0ae7fb20

Thank you :)