Add multi-finger swipe detection to GestureDetector

Add multi-finger swipe detection to GestureDetector

Add optional ET_GESTURE_MULTIFINGER_SWIPE detection to the unified
GestureDetector implemenation.  This is currently used only on Aura.

BUG=354397
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=266087

[jdduke (slow), 2014-04-18 17:26:08]

tdresser@: PTAL.  I may add another test (for >2 touch points), and I have yet
to try it out on a device, but I don't expect to have to change much.  I tried
to be faithful to the GestureSequence implementation (without the funny
min_velocity squaring).

[jdduke (slow), 2014-04-18 17:52:00]

I also considered adding some of the more subtle direction logic to
GestureProvider instead of GestureDetector, but I don't think the changes were
too invasive.

[tdresser, 2014-04-22 15:27:16]

Works great on Aura with unified GR.

I'd like to hear your thoughts on splitting out a
MultiFingerSwipeGestureDetector, but other than that (and a small nit) it's
good.

https://codereview.chromium.org/243403002/diff/40001/ui/events/gesture_detect...
File ui/events/gesture_detection/gesture_config_helper_aura.cc (right):

https://codereview.chromium.org/243403002/diff/40001/ui/events/gesture_detect...
ui/events/gesture_detection/gesture_config_helper_aura.cc:32:
config.minimum_swipe_direction_ratio =
This name isn't great, nor was the previous one.

We could rename it minimum_swipe_deviation_angle, and do a wee bit of trig?

Side note: we're caching these configuration values all over the place, which
will make getting about://gesture up and running with the unified GR a bit of a
pain. I'm not sure how we'll want to approach that.

https://codereview.chromium.org/243403002/diff/40001/ui/events/gesture_detect...
File ui/events/gesture_detection/gesture_detector.cc (right):

https://codereview.chromium.org/243403002/diff/40001/ui/events/gesture_detect...
ui/events/gesture_detection/gesture_detector.cc:220: const float vx1 =
velocity_tracker_.GetXVelocity(id1);
I was envisioning adding an independent object to recognize each new gesture
type we decided to add.

So we'd have a MultifingerSwipeGestureDetector, a TwoFingerTapGestureDetector,
etc.

I'm worried that the approach of extending the GestureDetector could spiral out
of control. 

That being said, this is a lot less code than that approach would require. Do
you think we should plan on just extending the existing GestureDetector?

I'm in favor of minimizing the logic in the GestureProvider - I don't think this
logic belongs there.

[tdresser, 2014-04-22 16:03:26]

LGTM, I'm ok with putting the logic in GestureDetector, as per offline
conversation.

[jdduke (slow), 2014-04-24 00:54:28]

tdresser@: Could you take a look at the trig? Thanks.  We'll probably want to
update ui::GestureConfiguration as well.

https://codereview.chromium.org/243403002/diff/40001/ui/events/gesture_detect...
File ui/events/gesture_detection/gesture_config_helper_aura.cc (right):

https://codereview.chromium.org/243403002/diff/40001/ui/events/gesture_detect...
ui/events/gesture_detection/gesture_config_helper_aura.cc:32:
config.minimum_swipe_direction_ratio =
On 2014/04/22 15:27:16, tdresser wrote:
> This name isn't great, nor was the previous one.
> 
> We could rename it minimum_swipe_deviation_angle, and do a wee bit of trig?
> 
> Side note: we're caching these configuration values all over the place, which
> will make getting about://gesture up and running with the unified GR a bit of
a
> pain. I'm not sure how we'll want to approach that.

Yeah that's better, though I'll probably cache the cotan(angle) value to get the
ratio when initializing the GestureDetector.

[tdresser, 2014-04-24 12:42:39]

LGTM, with a couple of nits.

I filed a bug to update GestureConfiguration:
https://code.google.com/p/chromium/issues/detail?id=366603

https://codereview.chromium.org/243403002/diff/60001/ui/events/gesture_detect...
File ui/events/gesture_detection/gesture_detector.cc (right):

https://codereview.chromium.org/243403002/diff/60001/ui/events/gesture_detect...
ui/events/gesture_detection/gesture_detector.cc:24: const float
kDegreesToRadians = 3.14159265f / 180.0f;
Might as well use M_PI here.

Note that you need a 
#ifdef _WIN32
#define _USE_MATH_DEFINES
#endif
for windows.

https://codereview.chromium.org/243403002/diff/60001/ui/events/gesture_detect...
File ui/events/gesture_detection/gesture_detector.h (right):

https://codereview.chromium.org/243403002/diff/60001/ui/events/gesture_detect...
ui/events/gesture_detection/gesture_detector.h:51: // Maximum angle of the swipe
from it's dominant component axis, between
Extremely nitful nit.
it's -> its

[jdduke (slow), 2014-04-24 15:13:31]

The CQ bit was checked by jdduke@chromium.org

[jdduke (slow), 2014-04-24 15:13:54]

https://codereview.chromium.org/243403002/diff/60001/ui/events/gesture_detect...
File ui/events/gesture_detection/gesture_detector.cc (right):

https://codereview.chromium.org/243403002/diff/60001/ui/events/gesture_detect...
ui/events/gesture_detection/gesture_detector.cc:24: const float
kDegreesToRadians = 3.14159265f / 180.0f;
On 2014/04/24 12:42:40, tdresser wrote:
> Might as well use M_PI here.
> 
> Note that you need a 
> #ifdef _WIN32
> #define _USE_MATH_DEFINES
> #endif
> for windows.

Done.

https://codereview.chromium.org/243403002/diff/60001/ui/events/gesture_detect...
File ui/events/gesture_detection/gesture_detector.h (right):

https://codereview.chromium.org/243403002/diff/60001/ui/events/gesture_detect...
ui/events/gesture_detection/gesture_detector.h:51: // Maximum angle of the swipe
from it's dominant component axis, between
On 2014/04/24 12:42:40, tdresser wrote:
> Extremely nitful nit.
> it's -> its

Done.

[commit-bot: I haz the power, 2014-04-24 15:14:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/243403002/80001

[commit-bot: I haz the power, 2014-04-24 16:45:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-24 16:45:52]

Try jobs failed on following builders:
  tryserver.chromium on win_chromium_rel

[jdduke (slow), 2014-04-24 18:00:15]

The CQ bit was checked by jdduke@chromium.org

[jdduke (slow), 2014-04-24 20:28:14]

The CQ bit was unchecked by jdduke@chromium.org

[jdduke (slow), 2014-04-24 20:28:19]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-04-24 21:11:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/243403002/100001

[commit-bot: I haz the power, 2014-04-24 22:46:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-24 22:46:47]

Try jobs failed on following builders:
  tryserver.chromium on win_chromium_rel

[jdduke (slow), 2014-04-24 22:49:52]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-04-24 22:51:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/243403002/100001

[commit-bot: I haz the power, 2014-04-25 02:25:34]

Change committed as 266087