Make debugger profiling honor deleted commands

Fixes http://code.google.com/p/skia/issues/detail?id=999

[DerekS, 2012-12-06 21:34:51]

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

https://codereview.appspot.com/6906043/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:196: for (++fCurCommand;
can we make this a while loop for clarity?

https://codereview.appspot.com/6906043/diff/1/src/core/SkPicturePlayback.cpp
File src/core/SkPicturePlayback.cpp (right):

https://codereview.appspot.com/6906043/diff/1/src/core/SkPicturePlayback.cpp#...
src/core/SkPicturePlayback.cpp:641: if (skipTo == SK_MaxU32) {
can you document that SK_MaxU32 is a signal that we have reached the end of the
command stream?

[robertphillips, 2012-12-07 13:22:29]

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

https://codereview.appspot.com/6906043/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:196: for (++fCurCommand;
On 2012/12/06 21:34:52, DerekS wrote:
> can we make this a while loop for clarity?

Done.

https://codereview.appspot.com/6906043/diff/1/src/core/SkPicturePlayback.cpp
File src/core/SkPicturePlayback.cpp (right):

https://codereview.appspot.com/6906043/diff/1/src/core/SkPicturePlayback.cpp#...
src/core/SkPicturePlayback.cpp:641: if (skipTo == SK_MaxU32) {
On 2012/12/06 21:34:52, DerekS wrote:
> can you document that SK_MaxU32 is a signal that we have reached the end of
the
> command stream?

Done.

[TomH, 2012-12-07 14:45:19]

LGTM

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

https://codereview.appspot.com/6906043/diff/5002/debugger/QT/SkDebuggerGUI.cp...
debugger/QT/SkDebuggerGUI.cpp:145: const SkTDArray<bool>& deletedCommands)
Ugh - well, if it's the best way, it's the best way. Is this the right name?

https://codereview.appspot.com/6906043/diff/5002/src/core/SkPicturePlayback.cpp
File src/core/SkPicturePlayback.cpp (right):

https://codereview.appspot.com/6906043/diff/5002/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:627: uint32_t off = it.draw();
Can we name this 'offset' just for the clarity? (Or something even longer, like
'nextOffset'?)

[robertphillips, 2012-12-07 15:15:40]

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

https://codereview.appspot.com/6906043/diff/5002/debugger/QT/SkDebuggerGUI.cp...
debugger/QT/SkDebuggerGUI.cpp:145: const SkTDArray<bool>& deletedCommands)
I don't know if it is the best way but we're in somewhat of a bind here. The
SkDebugCanvas knows about command deletion but is too slow for profiling. We
re-open the skp file with an SkTimedPicture to profile but it is only a shallow
wrapper (so as not to interfere with performance). This doesn't give us a lot of
options w.r.t. where to hang the deleted operation information.

I'm open to other names. The GUI drove the 'deleted' moniker but internally the
SkDebugCanvas uses 'isVisible' in each command. How about 'skipCommand'?

https://codereview.appspot.com/6906043/diff/5002/src/core/SkPicturePlayback.cpp
File src/core/SkPicturePlayback.cpp (right):

https://codereview.appspot.com/6906043/diff/5002/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:627: uint32_t off = it.draw();
I'm partial to skipTo. What do y'all think?

[DerekS, 2012-12-07 18:02:59]

lgtm.

both fSkipCommands and skipTo are fine with me

[robertphillips, 2012-12-07 20:49:16]

committed as r6713