[webkit-reviews] review denied: [Bug 58186] [chromium] Add lowpass filter and graph to fps indicator : [Attachment 89396] Patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Nat Duca
<nduca at chromium.org>'s request for review:
Bug 58186: [chromium] Add lowpass filter and graph to fps indicator
https://bugs.webkit.org/show_bug.cgi?id=58186

Attachment 89396: Patch
https://bugs.webkit.org/attachment.cgi?id=89396&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=89396&action=review

> Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplay.cpp:156
> +	       m_presentTimeHistoryInSec[(m_currentFrameNumber +
kPresentHistorySize - 2) % kPresentHistorySize];

this might read better if this second line were indented so that you get:

  double secForLastFrame = m_presentTimeHistoryInSec[...
			   m_presentTimeHistoryInSec[...

> Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplay.cpp:165
> +

nit: kill the extra new line here

> Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplay.cpp:188
> +    int graphLeft = (int)(textWidth + 3);

nit, sorry... you should use C++ style static_cast instead of C-style casts.

> Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplay.cpp:191
> +    double h = (double)height - 2;

ditto for the cast