Remove offsetcanvas from debugger

I'm proposing this as a superset of https://codereview.chromium.org/14103024/.

This fixes the problem Leon was encountering (op counts getting confused due to NOOPs) and removes some unnecessary code.

[scroggo, 2013-05-22 14:11:07]

https://codereview.chromium.org/15746003/diff/1/debugger/QT/SkDebuggerGUI.cpp
File debugger/QT/SkDebuggerGUI.cpp (right):

https://codereview.chromium.org/15746003/diff/1/debugger/QT/SkDebuggerGUI.cpp...
debugger/QT/SkDebuggerGUI.cpp:182: size_t fCurOffset;
Is this still being used?

https://codereview.chromium.org/15746003/diff/1/debugger/QT/SkDebuggerGUI.cpp...
debugger/QT/SkDebuggerGUI.cpp:186: virtual bool preDraw(int opNum, int type) {
SK_OVERRIDE.

Also, should these be inside #ifdef SK_DEVELOPER?

https://codereview.chromium.org/15746003/diff/1/debugger/QT/SkDebuggerGUI.cpp...
debugger/QT/SkDebuggerGUI.cpp:213: virtual void postDraw(int opNum) {
SK_OVERRIDE

https://codereview.chromium.org/15746003/diff/1/src/core/SkPicturePlayback.cpp
File src/core/SkPicturePlayback.cpp (right):

https://codereview.chromium.org/15746003/diff/1/src/core/SkPicturePlayback.cp...
src/core/SkPicturePlayback.cpp:628: bool SkPicturePlayback::preDraw(int opNum,
int type) {
I find opNum a little confusing. I was assuming that it meant a number
associated with that type of operation, as opposed to the index in the stream of
this instance of the operation.

Maybe that's just me though. opIndex is the only other name I can think of, and
it may be no clearer. I understood it once I saw opNum++ below.

[djsollen, 2013-05-22 14:35:37]

https://codereview.chromium.org/15746003/diff/1/debugger/QT/SkDebuggerGUI.cpp
File debugger/QT/SkDebuggerGUI.cpp (right):

https://codereview.chromium.org/15746003/diff/1/debugger/QT/SkDebuggerGUI.cpp...
debugger/QT/SkDebuggerGUI.cpp:919: SkTArray<SkString>* commands =
fDebugger.getDrawCommandsAsStrings();
if you just want the count use fDebugger.getSize() as it is cheaper.  Also this
currently leaks. :)

[robertphillips, 2013-05-22 18:51:28]

https://codereview.chromium.org/15746003/diff/1/debugger/QT/SkDebuggerGUI.cpp
File debugger/QT/SkDebuggerGUI.cpp (right):

https://codereview.chromium.org/15746003/diff/1/debugger/QT/SkDebuggerGUI.cpp...
debugger/QT/SkDebuggerGUI.cpp:182: size_t fCurOffset;
Nope - deleted.

https://codereview.chromium.org/15746003/diff/1/debugger/QT/SkDebuggerGUI.cpp...
debugger/QT/SkDebuggerGUI.cpp:186: virtual bool preDraw(int opNum, int type) {
Done.

https://codereview.chromium.org/15746003/diff/1/debugger/QT/SkDebuggerGUI.cpp...
debugger/QT/SkDebuggerGUI.cpp:213: virtual void postDraw(int opNum) {
On 2013/05/22 14:11:07, scroggo wrote:
> SK_OVERRIDE

Done.

https://codereview.chromium.org/15746003/diff/1/debugger/QT/SkDebuggerGUI.cpp...
debugger/QT/SkDebuggerGUI.cpp:919: SkTArray<SkString>* commands =
fDebugger.getDrawCommandsAsStrings();
Done-ish. I'm now using getSize and have moved the getDrawCommandsAsStrings call
back to where it was. I suspect the strings from the 'commands' array need to
stick around for the list control's sake.

https://codereview.chromium.org/15746003/diff/1/src/core/SkPicturePlayback.cpp
File src/core/SkPicturePlayback.cpp (right):

https://codereview.chromium.org/15746003/diff/1/src/core/SkPicturePlayback.cp...
src/core/SkPicturePlayback.cpp:628: bool SkPicturePlayback::preDraw(int opNum,
int type) {
I think opIndex is a bit clearer - done.

[scroggo, 2013-05-22 19:06:13]

lgtm

https://codereview.chromium.org/15746003/diff/1/debugger/QT/SkDebuggerGUI.cpp
File debugger/QT/SkDebuggerGUI.cpp (right):

https://codereview.chromium.org/15746003/diff/1/debugger/QT/SkDebuggerGUI.cpp...
debugger/QT/SkDebuggerGUI.cpp:919: SkTArray<SkString>* commands =
fDebugger.getDrawCommandsAsStrings();
On 2013/05/22 18:51:28, robertphillips wrote:
> Done-ish. I'm now using getSize and have moved the getDrawCommandsAsStrings
call
> back to where it was. I suspect the strings from the 'commands' array need to
> stick around for the list control's sake.

Agreed. Can you put commands into an SkAutoTDelete to free the memory (and
remove the comment)?

[robertphillips, 2013-05-23 13:22:07]

> Agreed. Can you put commands into an SkAutoTDelete to free the memory (and
> remove the comment)?

I'll handle memory leaks in a different CL since I think the problem is endemic.

[scroggo, 2013-05-23 13:23:43]

On 2013/05/23 13:22:07, robertphillips wrote:
> > Agreed. Can you put commands into an SkAutoTDelete to free the memory (and
> > remove the comment)?
> 
> I'll handle memory leaks in a different CL since I think the problem is
endemic.

Haha, no problem. LGTM

[robertphillips, 2013-05-23 13:26:28]

committed as r9253