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

[bugzilla-daemon at webkit.org]

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





--- Comment #16 from Scott Byer <scottbyer at chromium.org>  2011-06-03 09:17:03 PST ---
(In reply to comment #14)
> (From update of attachment 95843 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=95843&action=review
> 
> The Settings design confuses me too.  What functionality are you going for? Wouldn't things be a lot simpler if ScrollAnimatorSettingsChromium was a class nested in ScrollAnimatorChrome?  Are you trying to expose the scroll animator settings so that they can be modified from outside this code (i.e. via a UI or flags)?

Yes, I think eventually some of the settings will want to be modified base on values that the Browser knows (e.g., keyboard repeat rate). In addition, when we got down to the nitty-gritty of fine tuning things, I wanted to have a WebUI where I could fiddle with everything and see traces. For something like smooth scrolling, modify-compile-run is too slow.



> I agree with Adam.  Even if you do end up with a separate ScrolAnimatorSettings class, I think the enable/disable still fits as a boolean in WebCore::Settings .

That makes sense.

> 
> > Source/WebCore/platform/chromium/ScrollAnimatorChromium.cpp:382
> > +    const ScrollAnimatorSettingsChromium::Settings *input_settings;
> 
> input_settings -> inputSettings

Good catch. Not a fan of CamelCase (years of Microsoft Word auto-correcting Photoshop incorrectly), but I'm learning to cope.

> 
> > Source/WebCore/platform/chromium/ScrollAnimatorSettingsChromium.cpp:56
> > +    for (int i = 0; i < 5; ++i)
> 
> Better to augment the Type enum with a last value entry and replace the 5 here by the entry's name.

Will do.

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