Close pending save() calls in QT debugger to avoid saveLayer() corruption

This gets rid of both the afterimage in the canvas and the overlaid white rectangles that never render again.

I suspect it may interfere with properly stepping through playback (perhaps: pause somewhere, move around, step forward, move around, step forward, ...), but haven't yet triggered it. We *can not* allow saveLayer() calls to be unresolved, so I think we're going to have to abandon the current approach to stepping through and just replay the skp from the beginning to whatever the new index is.

[robertphillips, 2012-11-26 15:05:34]

LGTM but I defer to Derek.

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

https://codereview.appspot.com/6856076/diff/1/debugger/SkDebugCanvas.cpp#newc...
debugger/SkDebugCanvas.cpp:128: if (commandVector[i]->isVisible()) {
I wonder if it makes sense to just add the fOutstandingSaveCount parameter to
the execute call or make the execute calls take a debug canvas and add a
inc/dev-SaveState call to the debug canvas.

https://codereview.appspot.com/6856076/diff/1/debugger/SkDrawCommand.h
File debugger/SkDrawCommand.h (right):

https://codereview.appspot.com/6856076/diff/1/debugger/SkDrawCommand.h#newcode38
debugger/SkDrawCommand.h:38: /** Does nothing by default, but used by save() and
restore()-type
subclasses

[DerekS, 2012-11-27 14:26:16]

I understand the intent of the change, but this is going to break the ability to
step through a picture without a massive slowdown.

My first reaction is to use this logic to detect when we are in the middle of a
saveLayer sequence and copy the canvas pixels into a new texture/bitmap where we
can do the translate/scale operation to display to the user.  That would allow
us to keep working on the original canvas without having to replay everything at
each step.

The downside is that I don't think you will get to see the partial progress of
the save layer.

Any thoughts or opinions?

[DerekS, 2012-11-27 14:47:44]

Thanks to robert for pointing out to me the real behavior of the CL.  I got
ahead of myself reading the comments and didn't look at what the code was
actually doing.

So sorry for the confusion and feel free to ignore my earlier comments.

LGTM.

[TomH, 2012-11-27 15:54:59]

I think it *does* break stepping entirely, let alone the ability to do so
quickly. But I think being able to stop in the middle of a saveLayer() is more
important than (quick) stepping.

Patches welcome; I've got a stream of high-priority bugs that's kept me from
looking at it today, and likely for a few days yet, but there are Code Yellow
cases we can't investigate without fixing the underlying bug.

[DerekS, 2012-11-27 16:05:44]

Are you saying that this CL allows you to see the progress of the off-screen
layers canvas?  I'm not sure how that would be working if it is.

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

https://codereview.appspot.com/6856076/diff/1/debugger/SkDebugCanvas.cpp#newc...
debugger/SkDebugCanvas.cpp:99: } else {
how does this break stepping? It looks like this else clause will only get
executed when we go backwards and in that case we needed to draw from the
beginning anyway.

[TomH, 2012-11-27 16:07:19]

D'oh, you're right, it was only the prototype that would break stepping. This
might work. We'll see.

[TomH, 2012-11-27 16:09:59]

Closed with r6565.