[webkit-changes] [92639] trunk/Source

Mon, 08 Aug 2011 15:01:22 -0700

Title: [92639] trunk/Source
Revision
92639
Author
[email protected]
Date
2011-08-08 15:01:15 -0700 (Mon, 08 Aug 2011)

Log Message

Scroll animator changes to nail the framerate
https://bugs.webkit.org/show_bug.cgi?id=65645

Patch by Scott Byer <[email protected]> on 2011-08-08
Reviewed by James Robinson.

Source/WebCore:

Partial test in ScrollAnimatorNoneTest::Enabled.

* platform/ScrollAnimatorNone.cpp:
(WebCore::ScrollAnimatorNone::PerAxisData::PerAxisData):
(WebCore::ScrollAnimatorNone::PerAxisData::updateDataFromParameters):
(WebCore::ScrollAnimatorNone::PerAxisData::animateScroll):
(WebCore::ScrollAnimatorNone::ScrollAnimatorNone):
(WebCore::ScrollAnimatorNone::~ScrollAnimatorNone):
(WebCore::ScrollAnimatorNone::scroll):
(WebCore::ScrollAnimatorNone::scrollToOffsetWithoutAnimation):
(WebCore::ScrollAnimatorNone::animationTimerFired):
(WebCore::ScrollAnimatorNone::stopAnimationTimerIfNeeded):
* platform/ScrollAnimatorNone.h:

Source/WebKit/chromium:

* tests/ScrollAnimatorNoneTest.cpp:
(TEST):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (92638 => 92639)


--- trunk/Source/WebCore/ChangeLog	2011-08-08 21:53:39 UTC (rev 92638)
+++ trunk/Source/WebCore/ChangeLog	2011-08-08 22:01:15 UTC (rev 92639)
@@ -1,3 +1,24 @@
+2011-08-08  Scott Byer  <[email protected]>
+
+        Scroll animator changes to nail the framerate
+        https://bugs.webkit.org/show_bug.cgi?id=65645
+
+        Reviewed by James Robinson.
+
+        Partial test in ScrollAnimatorNoneTest::Enabled.
+
+        * platform/ScrollAnimatorNone.cpp:
+        (WebCore::ScrollAnimatorNone::PerAxisData::PerAxisData):
+        (WebCore::ScrollAnimatorNone::PerAxisData::updateDataFromParameters):
+        (WebCore::ScrollAnimatorNone::PerAxisData::animateScroll):
+        (WebCore::ScrollAnimatorNone::ScrollAnimatorNone):
+        (WebCore::ScrollAnimatorNone::~ScrollAnimatorNone):
+        (WebCore::ScrollAnimatorNone::scroll):
+        (WebCore::ScrollAnimatorNone::scrollToOffsetWithoutAnimation):
+        (WebCore::ScrollAnimatorNone::animationTimerFired):
+        (WebCore::ScrollAnimatorNone::stopAnimationTimerIfNeeded):
+        * platform/ScrollAnimatorNone.h:
+
 2011-08-08  Emil A Eklund  <[email protected]>
 
         Switch legacy flexbox to to new layout types

Modified: trunk/Source/WebCore/platform/ScrollAnimatorNone.cpp (92638 => 92639)


--- trunk/Source/WebCore/platform/ScrollAnimatorNone.cpp	2011-08-08 21:53:39 UTC (rev 92638)
+++ trunk/Source/WebCore/platform/ScrollAnimatorNone.cpp	2011-08-08 22:01:15 UTC (rev 92639)
@@ -48,11 +48,10 @@
 
 namespace WebCore {
 
-static double kTickTime = .0166;
+static double kFrameRate = 60;
+static double kTickTime = 1 / kFrameRate;
+static double kMinimumTimerInterval = .001;
 
-// This is used to set the timer delay - it needs to be slightly smaller than the tick count to leave some overhead.
-static double kAnimationTimerDelay = 0.015;
-
 PassOwnPtr<ScrollAnimator> ScrollAnimator::create(ScrollableArea* scrollableArea)
 {
     if (scrollableArea && scrollableArea->scrollAnimatorEnabled())
@@ -131,7 +130,6 @@
 
 ScrollAnimatorNone::PerAxisData::PerAxisData(ScrollAnimatorNone* parent, float* currentPosition)
     : m_currentPosition(currentPosition)
-    , m_animationTimer(parent, &ScrollAnimatorNone::animationTimerFired)
 {
     reset();
 }
@@ -194,7 +192,7 @@
         // FIXME: This should be the time from the event that got us here.
         m_startTime = currentTime - kTickTime / 2;
         m_startPosition = *m_currentPosition;
-        m_lastAnimationTime = currentTime;
+        m_lastAnimationTime = m_startTime;
     }
     m_startVelocity = m_currentVelocity;
 
@@ -240,8 +238,10 @@
 // FIXME: Add in jank detection trace events into this function.
 bool ScrollAnimatorNone::PerAxisData::animateScroll(double currentTime)
 {
-    // Get the current time; grabbing the current time once helps keep a consistent heartbeat.
     double lastScrollInterval = currentTime - m_lastAnimationTime;
+    if (lastScrollInterval < kMinimumTimerInterval)
+        return true;
+
     m_lastAnimationTime = currentTime;
 
     double deltaTime = currentTime - m_startTime;
@@ -274,13 +274,13 @@
     : ScrollAnimator(scrollableArea)
     , m_horizontalData(this, &m_currentPosX)
     , m_verticalData(this, &m_currentPosY)
+    , m_animationTimer(this, &ScrollAnimatorNone::animationTimerFired)
 {
 }
 
 ScrollAnimatorNone::~ScrollAnimatorNone()
 {
-    stopAnimationTimerIfNeeded(&m_horizontalData);
-    stopAnimationTimerIfNeeded(&m_verticalData);
+    stopAnimationTimerIfNeeded();
 }
 
 bool ScrollAnimatorNone::scroll(ScrollbarOrientation orientation, ScrollGranularity granularity, float step, float multiplier)
@@ -311,24 +311,21 @@
     if (!parameters.m_isEnabled)
         return ScrollAnimator::scroll(orientation, granularity, step, multiplier);
 
-    // This is an animatable scroll. Calculate the scroll delta.
-    PerAxisData* data = "" == VerticalScrollbar) ? &m_verticalData : &m_horizontalData;
+    // This is an animatable scroll. Set the animation in motion using the appropriate parameters.
+    float scrollableSize = static_cast<float>(m_scrollableArea->scrollSize(orientation));
 
-    float scrollableSize = static_cast<float>(m_scrollableArea->scrollSize(orientation));
-    bool result = data->updateDataFromParameters(orientation, step, multiplier, scrollableSize, WTF::currentTime(), &parameters);
-    if (!data->m_animationTimer.isActive()) {
-        result &= data->animateScroll(WTF::currentTime());
-        if (result)
-            data->m_animationTimer.startOneShot(kAnimationTimerDelay);
+    PerAxisData& data = "" == VerticalScrollbar) ? m_verticalData : m_horizontalData;
+    bool needToScroll = data.updateDataFromParameters(orientation, step, multiplier, scrollableSize, WTF::monotonicallyIncreasingTime(), &parameters);
+    if (needToScroll && !m_animationTimer.isActive()) {
+        m_startTime = data.m_startTime;
+        animationTimerFired(&m_animationTimer);
     }
-    notityPositionChanged();
-    return result;
+    return needToScroll;
 }
 
 void ScrollAnimatorNone::scrollToOffsetWithoutAnimation(const FloatPoint& offset)
 {
-    stopAnimationTimerIfNeeded(&m_horizontalData);
-    stopAnimationTimerIfNeeded(&m_verticalData);
+    stopAnimationTimerIfNeeded();
 
     m_horizontalData.reset();
     *m_horizontalData.m_currentPosition = offset.x();
@@ -343,21 +340,25 @@
 
 void ScrollAnimatorNone::animationTimerFired(Timer<ScrollAnimatorNone>* timer)
 {
-    double currentTime = WTF::currentTime();
-    if ((timer == &m_horizontalData.m_animationTimer) ?
-        m_horizontalData.animateScroll(currentTime) :
-        m_verticalData.animateScroll(currentTime))
-    {
-        double delta = WTF::currentTime() - currentTime;
-        timer->startOneShot(kAnimationTimerDelay - delta);
+    double currentTime = WTF::monotonicallyIncreasingTime();
+    double deltaToNextFrame = ceil((currentTime - m_startTime) * kFrameRate) / kFrameRate - (currentTime - m_startTime);
+
+    bool continueAnimation = false;
+    if (m_horizontalData.m_startTime && m_horizontalData.animateScroll(currentTime + deltaToNextFrame))
+        continueAnimation = true;
+    if (m_verticalData.m_startTime && m_verticalData.animateScroll(currentTime + deltaToNextFrame))
+        continueAnimation = true;
+    if (continueAnimation) {
+        double nextTimerInterval = max(kMinimumTimerInterval, deltaToNextFrame);
+        timer->startOneShot(nextTimerInterval);
     }
     notityPositionChanged();
 }
 
-void ScrollAnimatorNone::stopAnimationTimerIfNeeded(PerAxisData* data)
+void ScrollAnimatorNone::stopAnimationTimerIfNeeded()
 {
-    if (data->m_animationTimer.isActive())
-        data->m_animationTimer.stop();
+    if (m_animationTimer.isActive())
+        m_animationTimer.stop();
 }
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/platform/ScrollAnimatorNone.h (92638 => 92639)


--- trunk/Source/WebCore/platform/ScrollAnimatorNone.h	2011-08-08 21:53:39 UTC (rev 92638)
+++ trunk/Source/WebCore/platform/ScrollAnimatorNone.h	2011-08-08 22:01:15 UTC (rev 92639)
@@ -112,14 +112,16 @@
         Curve m_releaseCurve;
 
         ScrollbarOrientation m_orientation;
-        Timer<ScrollAnimatorNone> m_animationTimer;
     };
 
     void animationTimerFired(Timer<ScrollAnimatorNone>*);
-    void stopAnimationTimerIfNeeded(PerAxisData*);
+    void stopAnimationTimerIfNeeded();
 
     PerAxisData m_horizontalData;
     PerAxisData m_verticalData;
+
+    double m_startTime;
+    Timer<ScrollAnimatorNone> m_animationTimer;
 };
 
 } // namespace WebCore

Modified: trunk/Source/WebKit/chromium/ChangeLog (92638 => 92639)


--- trunk/Source/WebKit/chromium/ChangeLog	2011-08-08 21:53:39 UTC (rev 92638)
+++ trunk/Source/WebKit/chromium/ChangeLog	2011-08-08 22:01:15 UTC (rev 92639)
@@ -1,3 +1,13 @@
+2011-08-08  Scott Byer  <[email protected]>
+
+        Scroll animator changes to nail the framerate
+        https://bugs.webkit.org/show_bug.cgi?id=65645
+
+        Reviewed by James Robinson.
+
+        * tests/ScrollAnimatorNoneTest.cpp:
+        (TEST):
+
 2011-08-08  Dmitry Lomov  <[email protected]>
 
         https://bugs.webkit.org/show_bug.cgi?id=65778

Modified: trunk/Source/WebKit/chromium/tests/ScrollAnimatorNoneTest.cpp (92638 => 92639)


--- trunk/Source/WebKit/chromium/tests/ScrollAnimatorNoneTest.cpp	2011-08-08 21:53:39 UTC (rev 92638)
+++ trunk/Source/WebKit/chromium/tests/ScrollAnimatorNoneTest.cpp	2011-08-08 22:01:15 UTC (rev 92639)
@@ -79,8 +79,7 @@
 
     void reset()
     {
-        stopAnimationTimerIfNeeded(&m_horizontalData);
-        stopAnimationTimerIfNeeded(&m_verticalData);
+        stopAnimationTimerIfNeeded();
         m_currentPosX = 0;
         m_currentPosY = 0;
         m_horizontalData.reset();
@@ -93,57 +92,57 @@
 TEST(ScrollAnimatorEnabled, Enabled)
 {
     MockScrollableArea scrollableArea(true);
-    MockScrollAnimatorNone scrollAnimatorChromium(&scrollableArea);
+    MockScrollAnimatorNone scrollAnimatorNone(&scrollableArea);
 
     EXPECT_CALL(scrollableArea, scrollSize(_)).Times(AtLeast(1)).WillRepeatedly(Return(1000));
-    EXPECT_CALL(scrollableArea, setScrollOffset(_)).Times(AtLeast(1));
+    EXPECT_CALL(scrollableArea, setScrollOffset(_)).Times(3);
 
-    scrollAnimatorChromium.scroll(HorizontalScrollbar, ScrollByLine, 100, 1);
-    EXPECT_NE(100, scrollAnimatorChromium.currentX());
-    EXPECT_NE(0, scrollAnimatorChromium.currentX());
-    EXPECT_EQ(0, scrollAnimatorChromium.currentY());
-    scrollAnimatorChromium.reset();
+    scrollAnimatorNone.scroll(HorizontalScrollbar, ScrollByLine, 100, 1);
+    EXPECT_NE(100, scrollAnimatorNone.currentX());
+    EXPECT_NE(0, scrollAnimatorNone.currentX());
+    EXPECT_EQ(0, scrollAnimatorNone.currentY());
+    scrollAnimatorNone.reset();
 
-    scrollAnimatorChromium.scroll(HorizontalScrollbar, ScrollByPage, 100, 1);
-    EXPECT_NE(100, scrollAnimatorChromium.currentX());
-    EXPECT_NE(0, scrollAnimatorChromium.currentX());
-    EXPECT_EQ(0, scrollAnimatorChromium.currentY());
-    scrollAnimatorChromium.reset();
+    scrollAnimatorNone.scroll(HorizontalScrollbar, ScrollByPage, 100, 1);
+    EXPECT_NE(100, scrollAnimatorNone.currentX());
+    EXPECT_NE(0, scrollAnimatorNone.currentX());
+    EXPECT_EQ(0, scrollAnimatorNone.currentY());
+    scrollAnimatorNone.reset();
 
-    scrollAnimatorChromium.scroll(HorizontalScrollbar, ScrollByPixel, 4, 25);
-    EXPECT_NE(100, scrollAnimatorChromium.currentX());
-    EXPECT_NE(0, scrollAnimatorChromium.currentX());
-    EXPECT_EQ(0, scrollAnimatorChromium.currentY());
-    scrollAnimatorChromium.reset();
+    scrollAnimatorNone.scroll(HorizontalScrollbar, ScrollByPixel, 4, 25);
+    EXPECT_NE(100, scrollAnimatorNone.currentX());
+    EXPECT_NE(0, scrollAnimatorNone.currentX());
+    EXPECT_EQ(0, scrollAnimatorNone.currentY());
+    scrollAnimatorNone.reset();
 }
 
 TEST(ScrollAnimatorEnabled, Disabled)
 {
     MockScrollableArea scrollableArea(false);
-    MockScrollAnimatorNone scrollAnimatorChromium(&scrollableArea);
+    MockScrollAnimatorNone scrollAnimatorNone(&scrollableArea);
 
     EXPECT_CALL(scrollableArea, scrollSize(_)).Times(AtLeast(1)).WillRepeatedly(Return(1000));
-    EXPECT_CALL(scrollableArea, setScrollOffset(_)).Times(AtLeast(1));
+    EXPECT_CALL(scrollableArea, setScrollOffset(_)).Times(4);
 
-    scrollAnimatorChromium.scroll(HorizontalScrollbar, ScrollByLine, 100, 1);
-    EXPECT_EQ(100, scrollAnimatorChromium.currentX());
-    EXPECT_EQ(0, scrollAnimatorChromium.currentY());
-    scrollAnimatorChromium.reset();
+    scrollAnimatorNone.scroll(HorizontalScrollbar, ScrollByLine, 100, 1);
+    EXPECT_EQ(100, scrollAnimatorNone.currentX());
+    EXPECT_EQ(0, scrollAnimatorNone.currentY());
+    scrollAnimatorNone.reset();
 
-    scrollAnimatorChromium.scroll(HorizontalScrollbar, ScrollByPage, 100, 1);
-    EXPECT_EQ(100, scrollAnimatorChromium.currentX());
-    EXPECT_EQ(0, scrollAnimatorChromium.currentY());
-    scrollAnimatorChromium.reset();
+    scrollAnimatorNone.scroll(HorizontalScrollbar, ScrollByPage, 100, 1);
+    EXPECT_EQ(100, scrollAnimatorNone.currentX());
+    EXPECT_EQ(0, scrollAnimatorNone.currentY());
+    scrollAnimatorNone.reset();
 
-    scrollAnimatorChromium.scroll(HorizontalScrollbar, ScrollByDocument, 100, 1);
-    EXPECT_EQ(100, scrollAnimatorChromium.currentX());
-    EXPECT_EQ(0, scrollAnimatorChromium.currentY());
-    scrollAnimatorChromium.reset();
+    scrollAnimatorNone.scroll(HorizontalScrollbar, ScrollByDocument, 100, 1);
+    EXPECT_EQ(100, scrollAnimatorNone.currentX());
+    EXPECT_EQ(0, scrollAnimatorNone.currentY());
+    scrollAnimatorNone.reset();
 
-    scrollAnimatorChromium.scroll(HorizontalScrollbar, ScrollByPixel, 100, 1);
-    EXPECT_EQ(100, scrollAnimatorChromium.currentX());
-    EXPECT_EQ(0, scrollAnimatorChromium.currentY());
-    scrollAnimatorChromium.reset();
+    scrollAnimatorNone.scroll(HorizontalScrollbar, ScrollByPixel, 100, 1);
+    EXPECT_EQ(100, scrollAnimatorNone.currentX());
+    EXPECT_EQ(0, scrollAnimatorNone.currentY());
+    scrollAnimatorNone.reset();
 }
 
 class ScrollAnimatorNoneTest : public testing::Test {

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to