Bug 66258

Summary: ScrollAnimatorNone coasting implementation
Product: WebKit Reporter: Scott Byer <scottbyer>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, jamesr, sam, scottbyer, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Address review comments none

Scott Byer
Reported 2011-08-15 15:20:37 PDT
ScrollAnimatorNone coasting implementation
Attachments
Patch (50.93 KB, patch)
2011-08-15 16:06 PDT, Scott Byer 		no flags
DetailsFormatted DiffDiff
Patch (49.35 KB, patch)
2011-08-18 11:34 PDT, Scott Byer 		no flags
DetailsFormatted DiffDiff
Patch (49.23 KB, patch)
2011-08-18 12:02 PDT, Scott Byer 		no flags
DetailsFormatted DiffDiff
Address review comments (51.42 KB, patch)
2011-08-18 16:55 PDT, Scott Byer 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Scott Byer
Comment 1 2011-08-15 16:06:41 PDT
Created attachment 103972 [details] Patch
Scott Byer
Comment 2 2011-08-18 11:34:17 PDT
Created attachment 104371 [details] Patch
Scott Byer
Comment 3 2011-08-18 11:36:49 PDT
Rebased. James, if you have a chance, could you review this? It's slightly big, but is mostly tests. As always, feel free to point me at someone else.
Scott Byer
Comment 4 2011-08-18 12:02:11 PDT
Created attachment 104378 [details] Patch
James Robinson
Comment 5 2011-08-18 12:26:25 PDT
Comment on attachment 104378 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104378&action=review > Source/WebCore/platform/ScrollAnimatorNone.cpp:141 > + double t1 = std::min(t, 1 / 2.75); In WebKit, we prefer to put a 'using namespace std' at the top of the file and use the name unqualified. Where are all of these magical numbers coming from? Is it possible to give them useful symbolic names? They seem a little precise to be completely arbitrary. > Source/WebKit/chromium/tests/ScrollAnimatorNoneTest.cpp:416 > + for (double t = .25 ; t < 1 ; t += .25) { nit: odd spacing here, i wouldn't expect a space before the ;
Scott Byer
Comment 6 2011-08-18 16:55:07 PDT
Created attachment 104425 [details] Address review comments
Scott Byer
Comment 7 2011-08-23 14:33:50 PDT
PTAL, I've cleaned up the style issues and rebased. Thanks.
James Robinson
Comment 8 2011-08-23 17:38:43 PDT
Comment on attachment 104425 [details] Address review comments Looks great, thanks for explaining some of the math in comments.
WebKit Review Bot
Comment 9 2011-08-23 18:37:03 PDT
Comment on attachment 104425 [details] Address review comments Clearing flags on attachment: 104425 Committed r93682: <http://trac.webkit.org/changeset/93682>
WebKit Review Bot
Comment 10 2011-08-23 18:37:08 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.