[Webkit-unassigned] [Bug 61878] Smooth scrolling for Chromium

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Vangelis Kokkevis <vangelis at chromium.org>  2011-06-02 12:50:40 PST ---
(From update of attachment 95781)
View in context: https://bugs.webkit.org/attachment.cgi?id=95781&action=review

This looks pretty good to me (I'm not a WK reviewer, I'm afraid).  Some comments inline.  I think ideally we'd want to make this available on windows too by switching the gyp file to include this version of the scroll animator instead of scrollAnimatorWin.  Also, why not have it compiled in by default and rely on the settings flag alone to turn it on or off?

> Source/WebCore/page/Settings.cpp:189
> +    , m_enableScrollAnimator(false)

This variable isn't used.

> Source/WebCore/platform/chromium/ScrollAnimatorChromium.cpp:57
> +    Settings initial = { false, 0.25, Linear, 0.01, Linear, 0.01, false, false, Linear, 0, 0 };

Better to use ScrollAnimatorSettings::Settings to make it clear this isn't the WebCore::Settings class.  Here and below.

> Source/WebCore/platform/chromium/ScrollAnimatorChromium.cpp:402
> +        multiplier *= 1.5;

Magic values here and below should probably be constants somewhere, although not necessarily for this patch.

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