Bug 81398

Summary: [chromium] Tune fling physics curve.
Product: WebKit Reporter: W. James MacLean <wjmaclean>
Component: New BugsAssignee: W. James MacLean <wjmaclean>
Status: RESOLVED FIXED    
Severity: Normal CC: anicolao, davemoore, jamesr, rjkroege, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Velocity profile
none
Patch
none
Patch
none
Patch none

Description W. James MacLean 2012-03-16 13:40:12 PDT
[chromium] Tune fling physics curve. [Not for review yet]
Comment 1 W. James MacLean 2012-03-16 13:40:59 PDT
Created attachment 132364 [details]
Patch
Comment 2 WebKit Review Bot 2012-03-16 13:43:14 PDT
Attachment 132364 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 W. James MacLean 2012-03-16 13:44:22 PDT
Curve mod proposal #1: just increase initial run-out time. Feel free to play with the parameter ... this patch will break unit tests but please ignore for now.

I'll post an alternative shortly, based on a pseudo-parabolic curve.
Comment 4 W. James MacLean 2012-03-16 14:33:30 PDT
Created attachment 132381 [details]
Patch
Comment 5 W. James MacLean 2012-03-16 14:34:07 PDT
Comment on attachment 132381 [details]
Patch

This curve profile has ramp-up shown in attached PDF file.
Comment 6 W. James MacLean 2012-03-16 14:35:15 PDT
Created attachment 132382 [details]
Velocity profile

Velocity profile for curve in second patch.
Comment 7 WebKit Review Bot 2012-03-16 14:35:36 PDT
Attachment 132381 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/platform/TouchFlingPlatformGestureCurve.cpp:66:  Missing spaces around /  [whitespace/operators] [3]
Source/WebCore/platform/TouchFlingPlatformGestureCurve.cpp:72:  Missing spaces around /  [whitespace/operators] [3]
Source/WebCore/platform/TouchFlingPlatformGestureCurve.cpp:74:  Missing spaces around /  [whitespace/operators] [3]
Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 4 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 W. James MacLean 2012-03-16 16:38:51 PDT
Created attachment 132413 [details]
Patch
Comment 9 WebKit Review Bot 2012-03-16 16:40:37 PDT
Attachment 132413 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/platform/TouchFlingPlatformGestureCurve.cpp:66:  Missing spaces around /  [whitespace/operators] [3]
Source/WebCore/platform/TouchFlingPlatformGestureCurve.cpp:72:  Missing spaces around /  [whitespace/operators] [3]
Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 3 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Robert Kroeger 2012-03-16 16:48:11 PDT
it feels pretty right to me.
Comment 11 Build Bot 2012-03-16 17:13:56 PDT
Comment on attachment 132413 [details]
Patch

Attachment 132413 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/11967545
Comment 12 W. James MacLean 2012-03-19 10:32:45 PDT
Created attachment 132605 [details]
Patch
Comment 13 W. James MacLean 2012-03-19 11:05:57 PDT
Created attachment 132610 [details]
Patch
Comment 14 W. James MacLean 2012-03-19 11:06:31 PDT
Fixed issue with time scale factor.
Comment 15 Robert Kroeger 2012-03-19 11:31:07 PDT
I've tried it out and shown to anicolao. It seems like enough of an improvement that it's worth landing.
Comment 16 James Robinson 2012-03-19 11:33:04 PDT
Comment on attachment 132610 [details]
Patch

If we're using this for wheel flings can we call it the right thing, or alternately make this curve be the WheelFlingPlatformGestureCurve?
Comment 17 Robert Kroeger 2012-03-19 12:38:22 PDT
(In reply to comment #16)
> (From update of attachment 132610 [details])
> If we're using this for wheel flings can we call it the right thing, or alternately make this curve be the WheelFlingPlatformGestureCurve?

Shouldn't this be a TrackPadFlingPlatformGestureCurve? An actual physical wheel should have something much more like the WheelFlingPlatformGestureCurve implementation.
Comment 18 James Robinson 2012-03-19 12:39:45 PDT
Sure, that might be an even better name.  The more explicit the better, IMO.
Comment 19 James Robinson 2012-03-19 18:59:15 PDT
Comment on attachment 132610 [details]
Patch

Looks fine.  Please rename soon if you get a chance so this is less confusing to read.
Comment 20 WebKit Review Bot 2012-03-20 07:52:17 PDT
Comment on attachment 132610 [details]
Patch

Clearing flags on attachment: 132610

Committed r111395: <http://trac.webkit.org/changeset/111395>
Comment 21 WebKit Review Bot 2012-03-20 07:52:25 PDT
All reviewed patches have been landed.  Closing bug.
Comment 22 W. James MacLean 2012-03-20 09:32:58 PDT
(In reply to comment #19)
> (From update of attachment 132610 [details])
> Looks fine.  Please rename soon if you get a chance so this is less confusing to read.

Will upload re-naming patch by end of day.