Breakdown profile time by command type in the overview pane

This CL breaks down the per command profile results by type and reports the results in the overview pane. It also fixes a bug in the timing where the first "throw-away" pass was being counted in the timing totals.

[DerekS, 2012-11-19 14:36:30]

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.cpp
File debugger/QT/SkDebuggerGUI.cpp (right):

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:140: this->resetTimes();
most edits in this file appear to be over indented.

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:147: for (int i = 0; i < LAST_DRAWTYPE_ENUM+1;
++i) {
tabs

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:147: for (int i = 0; i < LAST_DRAWTYPE_ENUM+1;
++i) {
i < fTypeTimes.count()

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:189: fCurType = DRAW_POS_TEXT_H;
is there any way to fire an assert if we come across a type we don't know so
that if we add new ones in the future we will catch it here instead of getting
bad results.

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:252: void resetTimes() {
((SkTimedPicturePlayback*) fPlayback)->resetTimes(); }
are there cases that we won't have a SkTimePicturePlayback and crash?

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:291: #if 0
is this info inside this if block intended to ever be surfaced to the app?  If
not can we remove it?

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:333: #if 1
can we put a comment if this is only meant to be temporary until you have a way
to configure it in the GUI?

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.h
File debugger/QT/SkDebuggerGUI.h (right):

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.h#new...
debugger/QT/SkDebuggerGUI.h:313: Render 'pict' 'repeats' times tracking the cost
of each command
fix spacing and the comment is unclear.

https://codereview.appspot.com/6851073/diff/1/debugger/SkDebugCanvas.cpp
File debugger/SkDebugCanvas.cpp (right):

https://codereview.appspot.com/6851073/diff/1/debugger/SkDebugCanvas.cpp#newc...
debugger/SkDebugCanvas.cpp:154: commandString->push(new
SkString(commandVector[i]->toString()));
I know this wasn't your code, but can we make this return a SkTDArray<SkString>*
instead of putting the strings on the heap?

[robertphillips, 2012-11-19 20:27:59]

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.cpp
File debugger/QT/SkDebuggerGUI.cpp (right):

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:140: this->resetTimes();
On 2012/11/19 14:36:31, DerekS wrote:
> most edits in this file appear to be over indented.

Done.

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:147: for (int i = 0; i < LAST_DRAWTYPE_ENUM+1;
++i) {
On 2012/11/19 14:36:31, DerekS wrote:
> tabs

Done.

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:147: for (int i = 0; i < LAST_DRAWTYPE_ENUM+1;
++i) {
On 2012/11/19 14:36:31, DerekS wrote:
> i < fTypeTimes.count()

Done.

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:189: fCurType = DRAW_POS_TEXT_H;
There is an assert in setupOverviewText that will fire if a command is seen to
consume some time but doesn't appear in the debugger's known commands.

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:252: void resetTimes() {
((SkTimedPicturePlayback*) fPlayback)->resetTimes(); }
I have (arguably) made it a bit safer by disabling the default and copy ctors. A
user can still get themselves into trouble by calling swap, or endRecording
(neither of which we do in this file). I don't know if adding a generic "reset"
interface to SkPicture & SkPicturePlayback is warranted.

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:291: #if 0
On 2012/11/19 14:36:31, DerekS wrote:
> is this info inside this if block intended to ever be surfaced to the app?  If
> not can we remove it?

Done.

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:333: #if 1
On 2012/11/19 14:36:31, DerekS wrote:
> can we put a comment if this is only meant to be temporary until you have a
way
> to configure it in the GUI?

Done.

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.h
File debugger/QT/SkDebuggerGUI.h (right):

https://codereview.appspot.com/6851073/diff/1/debugger/QT/SkDebuggerGUI.h#new...
debugger/QT/SkDebuggerGUI.h:313: Render 'pict' 'repeats' times tracking the cost
of each command
On 2012/11/19 14:36:31, DerekS wrote:
> fix spacing and the comment is unclear.

Done.

https://codereview.appspot.com/6851073/diff/1/debugger/SkDebugCanvas.cpp
File debugger/SkDebugCanvas.cpp (right):

https://codereview.appspot.com/6851073/diff/1/debugger/SkDebugCanvas.cpp#newc...
debugger/SkDebugCanvas.cpp:154: commandString->push(new
SkString(commandVector[i]->toString()));
On 2012/11/19 14:36:31, DerekS wrote:
> I know this wasn't your code, but can we make this return a
SkTDArray<SkString>*
> instead of putting the strings on the heap?

Done - probably should've done this as a separate CL :(

[DerekS, 2012-11-19 20:37:40]

lgtm

[robertphillips, 2012-11-19 20:44:49]

committed as r6492