Page MenuHomePhabricator

efl/gesture: move Point_Data to eo and add methods to fetch it for recognizers
ClosedPublic

Authored by zmike on Jan 13 2020, 12:13 PM.

Details

Summary

this lets gesture framework track two touch points in order to distinguish between
successive presses and e.g., treat a simultaneous two finger tap as a single tap
gesture rather than two

it also simplifies some internal code and removes most hash lookups

Depends on D11084

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.
zmike created this revision.Jan 13 2020, 12:13 PM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

CHAN added a subscriber: CHAN.Jan 16 2020, 9:47 PM

@zmike Thank you for the commit.

Do you wanna make it supports up to two touches? ( It talking about "last_touch[2]")
A three-finger tapping case was supported... but I don't know if it's actually usable;

these two types to be public? (Efl_Gesture_Touch_Point_Info and Efl_Gesture_Touch_Point_Data)? (You moved the struct into .eot file.)

zmike added a comment.Jan 17 2020, 6:17 AM

This has several benefits:

  • We now have more definite data types for gesture recognizers to access (without needing to access private data)
  • We now have multiple touch points so we can filter out "bad" touch events for gestures which use only a single point (for example, in my testing on other devices, if I tap with one finger and then a second finger very quickly, only the first is used)
  • We can now directly access both touch points for gestures which require multiple points, such as zoom

I considered whether to make this a variable-sized array to be able to count all points, but it seemed like this was a good start which would allow us to begin untangling some of the more difficult bugs. We should continue discussing this, however, since I'm not sure what will be most useful for custom recognizers.

zmike updated this revision to Diff 28264.Jan 17 2020, 7:42 AM
zmike edited the summary of this revision. (Show Details)
zmike added a reviewer: CHAN.

update

segfaultxavi requested changes to this revision.Jan 17 2020, 8:40 AM
segfaultxavi added a subscriber: segfaultxavi.

I don't like that the two new structures are called Info and Data. They're synonyms!
Also, it's not clear what "point" and "touch" are. This should be explained.
Is a Point a finger (or mouse pointer)? Is a Touch a gesture being detected, which can be composed of multiple Points? (And which might end up not matching any efl gesture).
Then we definitely need better names... Touch and Point are synonyms too.
Point->Pointer?
Touch->Gesture_Candidate or Candidate?

Does Touch_Point_Data record the movement of a finger (or pointer) over the screen, and Touch_Point_Info represent the position of said finger (or pointer) at one point in time?
If so, I suggest these new names:
Info->Position, Location or Sample.
Data->Pointer since this data simply represents the pointer.

src/lib/evas/gesture/efl_canvas_gesture_types.eot
54

This would need to explain the units and the reference frame (when is time 0?).
Also, why is this useful, is this a global timer, or restarts with each gesture, etc.

This revision now requires changes to proceed.Jan 17 2020, 8:40 AM
zmike added a comment.Jan 17 2020, 9:22 AM

Yeah tbh I'm more interested in the conceptual parts of these changes and not the minutiae of docs and naming since it might change anyway. We can do our usual shedsmanship once the functionality and API are reasonably stable.

Data and Info are synonyms precisely because they were originally the same name and I needed something quick to throw in to avoid a conflict.

Understood, but without docs or proper names, I find it very hard to follow what is going on :)
Will try again.

CHAN added a comment.Jan 20 2020, 1:05 AM

@zmike thank you.

yes I understand the benefits about it.

how about this?
efl_canvas_gesture_touch_data_get(int n);

API that manages n in touch order.

e.g
A press-> B press-> C press-> D press (A (1) B (2) C (3) D (4))
C up ( A (1) B (2) D (3) )

@zmike The proposed API and the data structures split makes sense to me.

segfaultxavi resigned from this revision.Jan 20 2020, 1:15 AM
zmike added a comment.Jan 21 2020, 6:49 AM
In D11085#210730, @CHAN wrote:

@zmike thank you.

yes I understand the benefits about it.

how about this?
efl_canvas_gesture_touch_data_get(int n);

API that manages n in touch order.

e.g
A press-> B press-> C press-> D press (A (1) B (2) C (3) D (4))
C up ( A (1) B (2) D (3) )

This makes sense to me. The patch here was mostly intended to bootstrap having the ability to manage multiple points and start a discussion about how we wanted to manage them (there's comments about finger_list being broken by design in various places).

src/lib/evas/gesture/efl_canvas_gesture_types.eot
54

Yea this is just the same value that's used internally in canvas stuff, so we can put a ref for that probably.

zmike updated this revision to Diff 28341.Jan 21 2020, 11:56 AM
zmike added a reviewer: bu5hm4n.

rebase/update

zmike updated this revision to Diff 28386.Jan 22 2020, 6:36 AM

remove off-canvas blocking

zmike updated this revision to Diff 28437.Jan 23 2020, 12:24 PM

rework/rebase

zmike updated this revision to Diff 28550.Jan 27 2020, 11:15 AM

fix array usage because eina_array is dumb

segfaultxavi added inline comments.Jan 28 2020, 3:28 AM
src/lib/evas/gesture/efl_canvas_gesture_types.eot
54

Aaaaaand do you want to do that?

CHAN accepted this revision.Jan 28 2020, 4:35 AM

Looks good to me.

This revision is now accepted and ready to land.Jan 28 2020, 4:35 AM
zmike added inline comments.Jan 28 2020, 6:52 AM
src/lib/evas/gesture/efl_canvas_gesture_types.eot
54

At some point

zmike updated this revision to Diff 28658.Jan 29 2020, 11:20 AM

rebase/rework

Closed by commit rEFL28e89a5ac7e5: efl/gesture: move Point_Data to eo and add methods to fetch it for recognizers (authored by zmike, committed by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain WhyJan 30 2020, 8:12 AM
This revision was automatically updated to reflect the committed changes.