Attachment 119277 Details for Bug 73454 – Patch

[patch] Patch

bug-73454-20111214154124.patch (text/plain), 10.79 KB, created by Alex Nicolaou on 2011-12-14 12:41:26 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Nicolaou

Created: 2011-12-14 12:41:26 PST

Size: 10.79 KB

patch

obsolete

>Subversion Revision: 102590
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 68bb2e5d9cf71999956d5b356faadcc6f4115329..9582bb6de4636997d2eb3ac153499a9fd92f8d7d 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,37 @@
>+2011-12-14  Alex Nicolaou  <anicolao@chromium.org>
>+
>+        [chromium] When rendering goes idle, do not count that time against frame rate
>+        https://bugs.webkit.org/show_bug.cgi?id=73454
>+
>+        The FPS counter had a few issues with its reporting. The first
>+        3 swapbuffers are non-blocking and create FPS rates that are
>+        unrealistically high, throwing off the moving averages and
>+        introducing false spikes into the FPS graph. There was also no
>+        way to monitor the smoothness of the animation, or to focus in
>+        on a particular animation or transition.
>+
>+        This patch updates the FPS counter code so that bad data points
>+        are trimmed and not graphed; so that the graph itself is taller
>+        and more legible; so that there is a clear boundary between 40FPS
>+        and better; so that outlier data points are labelled with their
>+        frame rate so that you can see how slow the frame is that is
>+        causing the jank; and to keep statistics on frame rate between
>+        pauses in the activity in the UI so that you can trigger a
>+        transition and read off the average and standard deviation for
>+        that transition to judge it smooth enough or not.
>+
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        HUD is not testable in webkit.
>+
>+        * platform/graphics/chromium/cc/CCHeadsUpDisplay.cpp:
>+        (WebCore::CCHeadsUpDisplay::CCHeadsUpDisplay):
>+        (WebCore::CCHeadsUpDisplay::onFrameBegin):
>+        (WebCore::CCHeadsUpDisplay::drawHudContents):
>+        (WebCore::CCHeadsUpDisplay::drawFPSCounter):
>+        * platform/graphics/chromium/cc/CCHeadsUpDisplay.h:
>+
> 2011-12-12  Mary Wu  <mary.wu@torchmobile.com.cn>
> 
>         Upstream 3 files into WebCore/platform/blackberry
>diff --git a/Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplay.cpp b/Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplay.cpp
>index 708bcff8801b78881060b529858b289b95328aec..97e8ffe3a7b5779d577202bcae87d0e0788afdad 100644
>--- a/Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplay.cpp
>+++ b/Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplay.cpp
>@@ -50,6 +50,9 @@ using namespace std;
> CCHeadsUpDisplay::CCHeadsUpDisplay(LayerRendererChromium* owner)
>     : m_currentFrameNumber(1)
>     , m_filteredFrameTime(0)
>+    , m_averageFPS(0)
>+    , m_averageFPSCount(0)
>+    , m_varianceNumerator(0)
>     , m_layerRenderer(owner)
>     , m_useMapSubForUploads(owner->contextSupportsMapSub())
> {
>@@ -60,7 +63,7 @@ CCHeadsUpDisplay::CCHeadsUpDisplay(LayerRendererChromium* owner)
> 
>     FontDescription mediumFontDesc;
>     mediumFontDesc.setGenericFamily(FontDescription::MonospaceFamily);
>-    mediumFontDesc.setComputedSize(20);
>+    mediumFontDesc.setComputedSize(12);
> 
>     m_mediumFont = adoptPtr(new Font(mediumFontDesc, 0, 0));
>     m_mediumFont->update(0);
>@@ -77,9 +80,13 @@ CCHeadsUpDisplay::~CCHeadsUpDisplay()
> {
> }
> 
>+#define FRAME_INDEX(frame) ((frame) % kBeginFrameHistorySize)
> void CCHeadsUpDisplay::onFrameBegin(double timestamp)
> {
>-    m_beginTimeHistoryInSec[m_currentFrameNumber % kBeginFrameHistorySize] = timestamp;
>+    m_beginTimeHistoryInSec[FRAME_INDEX(m_currentFrameNumber)] = timestamp;
>+    double delta = m_beginTimeHistoryInSec[FRAME_INDEX(m_currentFrameNumber)] -
>+                   m_beginTimeHistoryInSec[FRAME_INDEX(m_currentFrameNumber - 1)];
>+    m_badFrame[FRAME_INDEX(m_currentFrameNumber)] = (delta < 1.0 / 70) || (delta > 5.0 / 60);
> }
> 
> void CCHeadsUpDisplay::onSwapBuffers()
>@@ -170,11 +177,11 @@ void CCHeadsUpDisplay::drawHudContents(GraphicsContext* ctx, const IntSize& hudS
>         ctx->fillRect(FloatRect(0, 0, hudSize.width(), hudSize.height()));
>     }
> 
>-    int fpsCounterHeight = m_mediumFont->fontMetrics().floatHeight() + 2;
>+    int fpsCounterHeight = m_mediumFont->fontMetrics().floatHeight() + 40;
>     int fpsCounterTop = 2;
>     int platformLayerTreeTop;
>     if (settings().showFPSCounter)
>-        platformLayerTreeTop = fpsCounterTop + fpsCounterHeight + 2;
>+        platformLayerTreeTop = fpsCounterTop + fpsCounterHeight;
>     else
>         platformLayerTreeTop = 0;
> 
>@@ -189,42 +196,76 @@ void CCHeadsUpDisplay::drawFPSCounter(GraphicsContext* ctx, int top, int height)
> {
>     // Note that since we haven't finished the current frame, the FPS counter
>     // actually reports the last frame's time.
>-    double secForLastFrame = m_beginTimeHistoryInSec[(m_currentFrameNumber + kBeginFrameHistorySize - 1) % kBeginFrameHistorySize] -
>-                             m_beginTimeHistoryInSec[(m_currentFrameNumber + kBeginFrameHistorySize - 2) % kBeginFrameHistorySize];
>+    double secForLastFrame = m_beginTimeHistoryInSec[FRAME_INDEX(m_currentFrameNumber - 1)] -
>+                             m_beginTimeHistoryInSec[FRAME_INDEX(m_currentFrameNumber - 2)];
>+    double secForThirdLastFrame = m_beginTimeHistoryInSec[FRAME_INDEX(m_currentFrameNumber - 3)] -
>+                                  m_beginTimeHistoryInSec[FRAME_INDEX(m_currentFrameNumber - 4)];
> 
>     // Filter the frame times to avoid spikes.
>     const float alpha = 0.1;
>     if (!m_filteredFrameTime) {
>         if (m_currentFrameNumber == 2)
>             m_filteredFrameTime = secForLastFrame;
>-    } else
>+    } else if (secForLastFrame > 0.5) {
>+        // don't filter the time if this is the first frame after a period of no drawing; pretend this
>+        // frame took as long as the previous frame draw instead
>+        m_filteredFrameTime = m_filteredFrameTime;
>+    } else {
>         m_filteredFrameTime = ((1.0 - alpha) * m_filteredFrameTime) + (alpha * secForLastFrame);
> 
>+        if (secForThirdLastFrame > 0.5 || m_averageFPSCount > 60*2) {
>+            // restart the frame statistics
>+            m_averageFPS = 0;
>+            m_averageFPSCount = 0;
>+            m_varianceNumerator = 0;
>+        } else if (!m_badFrame[FRAME_INDEX(m_currentFrameNumber - 1)]) {
>+            m_averageFPSCount++;
>+            double x = 1.0 / secForLastFrame;
>+            double deltaFromAverage = x - m_averageFPS;
>+            // Change with caution - numerics. http://en.wikipedia.org/wiki/Standard_deviation
>+            m_averageFPS = m_averageFPS + deltaFromAverage / m_averageFPSCount;
>+            m_varianceNumerator = m_varianceNumerator + deltaFromAverage * (x - m_averageFPS);
>+        }
>+    }
>+
>     // Create & measure FPS text.
>-    String text(String::format("FPS: %5.1f", 1.0 / m_filteredFrameTime));
>+    double stdDeviation = sqrt(m_varianceNumerator / m_averageFPSCount);
>+    String text(String::format("FPS: XXX.X (XX.X XXX.XXX)"));
>+    TextRun widthRun(text);
>+    float textWidth = m_mediumFont->width(widthRun) + 2.0f;
>+    text = String::format("FPS: %03.01f (%02.01f %03.03f)", 1.0 / m_filteredFrameTime, m_averageFPS, stdDeviation);
>     TextRun run(text);
>-    float textWidth = m_mediumFont->width(run) + 2.0f;
>     float graphWidth = kBeginFrameHistorySize;
> 
>     // Draw background.
>     ctx->setFillColor(Color(0, 0, 0, 255), ColorSpaceDeviceRGB);
>-    ctx->fillRect(FloatRect(2, top, textWidth + graphWidth, height));
>+    double fontHeight = m_mediumFont->fontMetrics().floatHeight() + 2;
>+    ctx->fillRect(FloatRect(2, top, textWidth, fontHeight));
>+    ctx->fillRect(FloatRect(2 + textWidth, top, graphWidth, height));
> 
>     // Draw FPS text.
>     if (m_filteredFrameTime) {
>-        ctx->setFillColor(Color(255, 0, 0), ColorSpaceDeviceRGB);
>-        ctx->drawText(*m_mediumFont, run, IntPoint(3, top + height - 6));
>+        ctx->setFillColor(Color(200, 200, 200), ColorSpaceDeviceRGB);
>+        ctx->drawText(*m_mediumFont, run, IntPoint(3, top + fontHeight - 4));
>     }
> 
>     // Draw FPS graph.
>     const double loFPS = 0.0;
>-    const double hiFPS = 120.0;
>+    const double hiFPS = 80.0;
>     ctx->setStrokeStyle(SolidStroke);
>+    ctx->setFillColor(Color(154, 205, 50), ColorSpaceDeviceRGB);
>+    ctx->fillRect(FloatRect(2 + textWidth, top, graphWidth, height / 2));
>+    ctx->setFillColor(Color(255, 250, 205), ColorSpaceDeviceRGB);
>+    ctx->fillRect(FloatRect(2 + textWidth, top + height / 2, graphWidth, height / 2));
>     ctx->setStrokeColor(Color(255, 0, 0), ColorSpaceDeviceRGB);
>+    ctx->setFillColor(Color(255, 0, 0), ColorSpaceDeviceRGB);
>     int graphLeft = static_cast<int>(textWidth + 3);
>     IntPoint prev(-1, 0);
>     int x = 0;
>     double h = static_cast<double>(height - 2);
>+    const double noOutlierFound = 100;
>+    double minimumOutlier = noOutlierFound;
>+    IntPoint outlierPosition;
>     for (int i = m_currentFrameNumber % kBeginFrameHistorySize; i != (m_currentFrameNumber - 1) % kBeginFrameHistorySize; i = (i + 1) % kBeginFrameHistorySize) {
>         int j = (i + 1) % kBeginFrameHistorySize;
>         double fps = 1.0 / (m_beginTimeHistoryInSec[j] - m_beginTimeHistoryInSec[i]);
>@@ -234,11 +275,23 @@ void CCHeadsUpDisplay::drawFPSCounter(GraphicsContext* ctx, int top, int height)
>         if (p > 1)
>             p = 1;
>         IntPoint cur(graphLeft + x, 1 + top + p*h);
>-        if (prev.x() != -1)
>-            ctx->drawLine(prev, cur);
>-        prev = cur;
>+        if (!m_badFrame[j]) {
>+            if (prev.x() != -1 && !m_badFrame[j]) {
>+                ctx->drawLine(prev, cur);
>+                if (fps > 1.0 && fps < 40 && fps < minimumOutlier && abs(fps - m_averageFPS) > 6*stdDeviation && stdDeviation > 0.01 && !isnan(stdDeviation)) {
>+                    minimumOutlier = fps;
>+                    outlierPosition = cur;
>+                }
>+            }
>+            prev = cur;
>+        }
>         x += 1;
>     }
>+    if (minimumOutlier != noOutlierFound) {
>+        String text(String::format("%2.1f", ((int)minimumOutlier*10) / 10.0));
>+        TextRun run(text);
>+        ctx->drawText(*m_mediumFont, run, IntPoint(outlierPosition.x(), outlierPosition.y() + fontHeight - 4));
>+    }
> }
> 
> void CCHeadsUpDisplay::drawPlatformLayerTree(GraphicsContext* ctx, int top)
>diff --git a/Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplay.h b/Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplay.h
>index 05cb20d526979859268e41774fcaa4a4ce1ba2a4..025ee7a38b099b88fc0006b795ea06592315f1f0 100644
>--- a/Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplay.h
>+++ b/Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplay.h
>@@ -70,6 +70,9 @@ private:
>     int m_currentFrameNumber;
> 
>     double m_filteredFrameTime;
>+    double m_averageFPS;
>+    int m_averageFPSCount;
>+    double m_varianceNumerator;
> 
>     OwnPtr<ManagedTexture> m_hudTexture;
> 
>@@ -77,6 +80,7 @@ private:
> 
>     static const int kBeginFrameHistorySize = 64;
>     double m_beginTimeHistoryInSec[kBeginFrameHistorySize];
>+    bool m_badFrame[kBeginFrameHistorySize];
> 
>     OwnPtr<Font> m_smallFont;
>     OwnPtr<Font> m_mediumFont;

Actions: View | Formatted Diff | Diff

Attachments on bug 73454: 119277 | 124885 | 125159 | 130531 | 130539 | 136629 | 136634 | 136696