requestAnimationFrame on Mac fires at 60fps even when drawing is much slower
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Dec 2011 23:55:11 +0000 (23:55 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Dec 2011 23:55:11 +0000 (23:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=74964

Reviewed by Chris Marrin.

On Mac requestAnimationFrame uses a CVDisplayLink, sending notifications
from the display link thread to the main thread that the display link fired.
However, there was no throttling on these notifications; if processing an event
took a long time, notifications would pile up, and then get handled after
the slow event completed.

This would cause JS animations which animate by changing style to report
60fps when their display framerate was much lower.

Fix by throttling notifications from the display link thread to the web
thread; if the previous event hasn't completed yet, don't send any new ones.

No new tests, since testing this runtime behavior is hard.

* platform/graphics/DisplayRefreshMonitor.cpp:
(WebCore::DisplayRefreshMonitor::DisplayRefreshMonitor):
(WebCore::DisplayRefreshMonitor::refreshDisplayOnMainThread):
(WebCore::DisplayRefreshMonitor::notifyClients): Factored out of the
static refreshDisplayOnMainThread method so we can use 'this'.
* platform/graphics/DisplayRefreshMonitor.h:
* platform/graphics/mac/DisplayRefreshMonitorMac.cpp:
(WebCore::DisplayRefreshMonitor::displayLinkFired):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@103361 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp
Source/WebCore/platform/graphics/DisplayRefreshMonitor.h
Source/WebCore/platform/graphics/mac/DisplayRefreshMonitorMac.cpp

index ad57d81..7c1835f 100644 (file)
@@ -1,3 +1,33 @@
+2011-12-20  Simon Fraser  <simon.fraser@apple.com>
+
+        requestAnimationFrame on Mac fires at 60fps even when drawing is much slower
+        https://bugs.webkit.org/show_bug.cgi?id=74964
+
+        Reviewed by Chris Marrin.
+        
+        On Mac requestAnimationFrame uses a CVDisplayLink, sending notifications
+        from the display link thread to the main thread that the display link fired.
+        However, there was no throttling on these notifications; if processing an event
+        took a long time, notifications would pile up, and then get handled after
+        the slow event completed.
+        
+        This would cause JS animations which animate by changing style to report
+        60fps when their display framerate was much lower.
+        
+        Fix by throttling notifications from the display link thread to the web
+        thread; if the previous event hasn't completed yet, don't send any new ones.
+
+        No new tests, since testing this runtime behavior is hard.
+
+        * platform/graphics/DisplayRefreshMonitor.cpp:
+        (WebCore::DisplayRefreshMonitor::DisplayRefreshMonitor):
+        (WebCore::DisplayRefreshMonitor::refreshDisplayOnMainThread):
+        (WebCore::DisplayRefreshMonitor::notifyClients): Factored out of the
+        static refreshDisplayOnMainThread method so we can use 'this'.
+        * platform/graphics/DisplayRefreshMonitor.h:
+        * platform/graphics/mac/DisplayRefreshMonitorMac.cpp:
+        (WebCore::DisplayRefreshMonitor::displayLinkFired):
+
 2011-12-20  Greg Billock  <gbillock@google.com>
 
         [Coverity] Fix leak in V8HTMLDocument
index 1f7b37a..23c3f72 100644 (file)
@@ -56,6 +56,7 @@ DisplayRefreshMonitor::DisplayRefreshMonitor(PlatformDisplayID displayID)
     : m_timestamp(0)
     , m_active(true)
     , m_scheduled(false)
+    , m_previousFrameDone(true)
     , m_displayID(displayID)
 #if PLATFORM(MAC)
     , m_displayLink(0)
@@ -66,18 +67,27 @@ DisplayRefreshMonitor::DisplayRefreshMonitor(PlatformDisplayID displayID)
 void DisplayRefreshMonitor::refreshDisplayOnMainThread(void* data)
 {
     DisplayRefreshMonitor* monitor = static_cast<DisplayRefreshMonitor*>(data);
+    monitor->notifyClients();
+}
 
+void DisplayRefreshMonitor::notifyClients()
+{
     double timestamp;
     {
-        MutexLocker lock(monitor->m_mutex);
-        monitor->m_scheduled = false;
-        timestamp = monitor->m_timestamp;
+        MutexLocker lock(m_mutex);
+        m_scheduled = false;
+        timestamp = m_timestamp;
     }
 
-    for (size_t i = 0; i < monitor->m_clients.size(); ++i)
-        monitor->m_clients[i]->fireDisplayRefreshIfNeeded(timestamp);
+    for (size_t i = 0; i < m_clients.size(); ++i)
+        m_clients[i]->fireDisplayRefreshIfNeeded(timestamp);
+
+    {
+        MutexLocker lock(m_mutex);
+        m_previousFrameDone = true;
+    }
 }
+
 DisplayRefreshMonitorManager* DisplayRefreshMonitorManager::sharedManager()
 {
     DEFINE_STATIC_LOCAL(DisplayRefreshMonitorManager, manager, ());
index 5038071..79df0d1 100644 (file)
@@ -96,9 +96,12 @@ public:
     PlatformDisplayID displayID() const { return m_displayID; }
 
 private:
+    void notifyClients();
+    
     double m_timestamp;
     bool m_active;
     bool m_scheduled;
+    bool m_previousFrameDone;
     PlatformDisplayID m_displayID;
     DisplayRefreshMonitorManager* m_manager;
     Mutex m_mutex;
index a259051..b0838e1 100644 (file)
@@ -87,9 +87,11 @@ bool DisplayRefreshMonitor::requestRefreshCallback()
 void DisplayRefreshMonitor::displayLinkFired(double nowSeconds, double outputTimeSeconds)
 {
     MutexLocker lock(m_mutex);
-    if (!m_scheduled)
+    if (!m_scheduled || !m_previousFrameDone)
         return;
 
+    m_previousFrameDone = false;
+
     double webKitNow = currentTime();
     m_timestamp = webKitNow - nowSeconds + outputTimeSeconds;