Add an additional fling curve parametrization specific to touchscreen fling.
Created attachment 162545 [details] Patch
Comment on attachment 162545 [details] Patch jamesr@: please take a look.
Comment on attachment 162545 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162545&action=review > Source/WebCore/platform/chromium/support/PlatformGestureCurveFactory.cpp:54 > + return WebCore::TouchpadFlingPlatformGestureCurve::createForTouchScreen(point, cumulativeScroll); Touchpad...ForTouchScreen? What does that mean? It doesn't seem sensical to create a Touchpad fling curve for a touch screen. I think what you really want is another fling curve type, if you want to be consistent with the existing curve type hierarchy. Or refactor the curve stuff to make more sense.
(In reply to comment #3) > (From update of attachment 162545 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=162545&action=review > > > Source/WebCore/platform/chromium/support/PlatformGestureCurveFactory.cpp:54 > > + return WebCore::TouchpadFlingPlatformGestureCurve::createForTouchScreen(point, cumulativeScroll); > > Touchpad...ForTouchScreen? What does that mean? It doesn't seem sensical to create a Touchpad fling curve for a touch screen. > > I think what you really want is another fling curve type, if you want to be consistent with the existing curve type hierarchy. Or refactor the curve stuff to make more sense. What if we renamed TouchpadFlingPlatformGestureCurve to TouchFlingPlatformGestureCurve and had functions TouchFlingPlatformGestureCurve::createForTouchPad() TouchFlingPlatformGestureCurve::createForTouchScreen() ?
That sounds better.
sounds good to me. I'll update the patch.
Created attachment 162824 [details] Patch
please take another look.
Comment on attachment 162824 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162824&action=review > Source/WebCore/platform/TouchFlingPlatformGestureCurve.cpp:69 > +// the in-flight patch for https://bugs.webkit.org/show_bug.cgi?id=81663 . I know you're just moving this code but this FIXME and comment are sad. https://bugs.webkit.org/show_bug.cgi?id=81663 was abandoned a while ago, do we still need these extra parameters? > Source/WebCore/platform/TouchFlingPlatformGestureCurve.h:56 > + float m_coeffs[5]; No abbreviations in webkit > Source/WebCore/platform/TouchFlingPlatformGestureCurve.h:61 > + static const int m_maxSearchIterations; Statics shouldn't have the m_ prefix. This probably doesn't should be a member at all - just make it a static const local in the function it's being used in, or a file-static if it's used in multiple places
Comment on attachment 162824 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162824&action=review >> Source/WebCore/platform/TouchFlingPlatformGestureCurve.cpp:69 >> +// the in-flight patch for https://bugs.webkit.org/show_bug.cgi?id=81663 . > > I know you're just moving this code but this FIXME and comment are sad. https://bugs.webkit.org/show_bug.cgi?id=81663 was abandoned a while ago, do we still need these extra parameters? True. Cleaned up in the landed patch. >> Source/WebCore/platform/TouchFlingPlatformGestureCurve.h:56 >> + float m_coeffs[5]; > > No abbreviations in webkit Fixed in landed patch. And I removed the excess coefficients. >> Source/WebCore/platform/TouchFlingPlatformGestureCurve.h:61 >> + static const int m_maxSearchIterations; > > Statics shouldn't have the m_ prefix. This probably doesn't should be a member at all - just make it a static const local in the function it's being used in, or a file-static if it's used in multiple places Done in landed patch.
Created attachment 162867 [details] Patch
Comment on attachment 162867 [details] Patch Clearing flags on attachment: 162867 Committed r127924: <http://trac.webkit.org/changeset/127924>
All reviewed patches have been landed. Closing bug.