[Webkit-unassigned] [Bug 67930] Implement flick gesture in Chromium Gesture Recognizer

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=67930





--- Comment #13 from Varun Jain <varunjain at chromium.org>  2011-09-29 10:12:17 PST ---
(In reply to comment #9)
> (From update of attachment 109088 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=109088&action=review
> 
> > Source/WebCore/platform/chromium/GestureRecognizerChromium.cpp:41
> > +static const float minFlickSpeed = 300.f;
> 
> I think recent on-device experience suggests that this should be higher.
> 
> > Source/WebCore/platform/chromium/GestureRecognizerChromium.cpp:105
> > +    return sqrt(m_xVelocity * m_xVelocity + m_yVelocity * m_yVelocity) > minFlickSpeed;
> 
> velocities are float. so use sqrtf

removed sqrt

> 
> > Source/WebCore/platform/chromium/GestureRecognizerChromium.h:96
> > +    IntPoint m_lastTouchPosition;
> 
> good hygiene would requires these to have initializers and be  included in the reset.
> 

done

> > Source/WebKit/chromium/tests/InnerGestureRecognizerTest.cpp:580
> > +struct TouchPointAndEvent {
> 
> I wouldn't have done this like this. I would have just had an array  of structs. But maybe this is nicer. It seems more complex than it needs to be.
> 
> I would have done: 
> static EventBlah* foo[] = { { ... }, { ... } };
> for (int i = 0; i < sizeof(foo) / sizeof(EventBlah*); i++) { /* do stuff */ }
> 
> > Source/WebKit/chromium/tests/InnerGestureRecognizerTest.cpp:583
> > +        : m_point(x, y, state),

I think I am missing something. What we have is an array of touch sequences where each sequence is composed of touch events. Each event is defined as a touchPoint and TouchEvent... so thats what the data structures represent. Do you have a better data structure?


> 
> the , goes under the : I think

Done

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.