[Webkit-unassigned] [Bug 91694] [chromium] Add droppedFrameCount to renderingStats.

[bugzilla-daemon at webkit.org]

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





--- Comment #6 from Dave Tu <dtu at chromium.org>  2012-07-19 15:03:04 PST ---
(From update of attachment 153136)
View in context: https://bugs.webkit.org/attachment.cgi?id=153136&action=review

>> Source/Platform/chromium/public/WebRenderingStats.h:33
>> +    int screenFrameCount;
> 
> As you note, by renaming these, you're going to break chrome when this lands. So, I suggest you put a comment saying // FIXME: when we land crbug.com/137790, rename this to animationFrameCount. Your focus here is on adding the droppedFrameCount in a hurry 'cause we really really desperately need it. We çan fix up naming and the complexities of renaming as that mentioned bug #

Okay.

>> Source/WebCore/platform/graphics/chromium/cc/CCFrameRateCounter.cpp:37
>> +const double CCFrameRateCounter::kDroppedFrameTime = 1.0 / 50.0;
> 
> So, whats the rationale behind 20ms? Is this what tonyg uses in the perf.py test?
> 
> You should file a followup bug that determines this based on the monitor refresh rate. I'm okay with it being fixed for v0. We're adding this value to CCThreadProxy very very soon. If FPSCounter was constructed with a "displayRefreshRate" value and had a mutation method setDisplayRefreshRate, the proxy could set it. But again, dont do it now, do it later. :)
> 
> http://code.google.com/p/chromium/issues/detail?id=129674

There's some noise in the frame times, so it should be slightly higher than 16.67 (which is what is in perf.py). The actual number is arbitrary, though. 1/55? 1/59?

Okay, sounds good; I'll add a FIXME here as well.

>> Source/WebCore/platform/graphics/chromium/cc/CCFrameRateCounter.cpp:45
>> +inline double CCFrameRateCounter::frameInterval(int frameNumber) const
> 
> This is elegant. Nicely done.

Thanks!

>> Source/WebCore/platform/graphics/chromium/cc/CCFrameRateCounter.cpp:-46
>> -    return safeMod(frame, kTimeStampHistorySize);
> 
> what if frameNumber == 0?

safeMod works on -1, see comment in line 39.

>> Source/WebCore/platform/graphics/chromium/cc/CCFrameRateCounter.cpp:70
>> +    if (!isBadFrameInterval(delta) && delta > kDroppedFrameTime)
> 
> Why are we checking isBadFrameInteravl()? Those are defined as really short frames, iirw? That wouldnt' count as a dropped frame anyways...

Both really short frames and really long frames, as in when rendering goes idle. Those idle frames should be ignored.

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