Fixing how deferred canvas purges itself when a clear is recoreded.

Fixing how deferred canvas purges itself when a clear is recoreded.

This fixes performance because the old code was not reconstructing the clip
state correctly. This was causing a major performance degradation in the 
Galactic IE testdrive demo. This fix also enbles the purge
on clear optimization when there is saved state on the matrix/clip stack.
The approach taken to solve the problem consists in purging by running the
playback silently. The previous approach was tearing down and
restarting the gpipe, which required reconstructing state, which is fragile
and hard to do correctly, and has the side effect of clearing the bitmap heap
and the flattened dictionary.

Note: This CL is expected to slightly degrade performance of the 
deferred_canvas_record bench, which uses the skip on clear optimization.
This is because a silent playback takes more time that just destroying
the SkGPipe. Correctness trumps performance.

BUG=http://code.google.com/p/chromium/issues/detail?id=146178
Committed: https://code.google.com/p/skia/source/detail?r=5627

[junov1, 2012-09-19 21:10:46]

PTAL

[Stephen White, 2012-09-19 21:18:34]

https://codereview.appspot.com/6531048/diff/1/src/utils/SkDeferredCanvas.cpp
File src/utils/SkDeferredCanvas.cpp (right):

https://codereview.appspot.com/6531048/diff/1/src/utils/SkDeferredCanvas.cpp#...
src/utils/SkDeferredCanvas.cpp:154: void nullPlayback();
Drive-by bikeshed paintsplatter:  maybe this should be a verb phrase, rather
than a noun phrase, since it actively does something; suggest
"playbackToNullDevice" or "playbackToNull" or "doNullPlayback" or ...

https://codereview.appspot.com/6531048/diff/1/src/utils/SkDeferredCanvas.cpp#...
src/utils/SkDeferredCanvas.cpp:412: void DeferredDevice::contentsCleared() {
Not new to this patch, but this one is also named by when it's called, rather
than by what it does, which would be hard for a n00b to grok.  Maybe
playbackToNullIfPossible()?  (Ok, that's pretty horrible, but you can riff on
that).  Worst case, onContentsCleared()?

[reed1, 2012-09-19 21:20:41]

adding leon

[junov1, 2012-09-19 21:29:48]

On 2012/09/19 21:18:34, Stephen White wrote:
> Maybe playbackToNullIfPossible()?  (Ok, that's pretty horrible, but you can
riff on
> that).  Worst case, onContentsCleared()?

How about "skipDeferred", the faster sibling of "flushDeferred"?

[Stephen White, 2012-09-19 21:33:52]

On 2012/09/19 21:29:48, junov1 wrote:
> On 2012/09/19 21:18:34, Stephen White wrote:
> > Maybe playbackToNullIfPossible()?  (Ok, that's pretty horrible, but you can
> riff on
> > that).  Worst case, onContentsCleared()?
> 
> How about "skipDeferred", the faster sibling of "flushDeferred"?

sounds good, although they're both missing a noun (am I a grammar nazi today or
what).  skipDeferredWhat?

[junov1, 2012-09-19 21:38:07]

On 2012/09/19 21:33:52, Stephen White wrote:
> skipDeferredWhat?

Commands!  You know what, let me do those in another CL

[Leon, 2012-09-19 21:59:34]

Some nits, but otherwise lgtm

https://codereview.appspot.com/6531048/diff/1/src/utils/SkDeferredCanvas.cpp
File src/utils/SkDeferredCanvas.cpp (left):

https://codereview.appspot.com/6531048/diff/1/src/utils/SkDeferredCanvas.cpp#...
src/utils/SkDeferredCanvas.cpp:392: // TODO: find a way to transfer the state
stack and layers
+1 for deleting code and fixing a TODO in the process!

https://codereview.appspot.com/6531048/diff/1/src/utils/SkDeferredCanvas.cpp#...
src/utils/SkDeferredCanvas.cpp:406: this->endRecording();
It looks like we can remove DeferredDevice::endRecording() and
DeferredPipeController::reset() now.

https://codereview.appspot.com/6531048/diff/1/src/utils/SkDeferredCanvas.cpp
File src/utils/SkDeferredCanvas.cpp (right):

https://codereview.appspot.com/6531048/diff/1/src/utils/SkDeferredCanvas.cpp#...
src/utils/SkDeferredCanvas.cpp:196: SkRefCnt_SafeAssign(fPlaybackDevice,
canvas->getDevice());
Since fPlaybackDevice is only used by ::nullPlayback(), I might prefer that we
just use a local variable in that function. On the other hand, that would mean
extra ref()s and unref()s.

https://codereview.appspot.com/6531048/diff/1/src/utils/SkDeferredCanvas.cpp#...
src/utils/SkDeferredCanvas.cpp:221: fPlaybackDevice->height());
should align with SkBitmap::kNo_Config

https://codereview.appspot.com/6531048/diff/1/src/utils/SkDeferredCanvas.cpp#...
src/utils/SkDeferredCanvas.cpp:225: playback();
this->playback();

[Stephen White, 2012-09-19 22:00:58]

LGTM

[junov1, 2012-09-19 23:52:23]

On 2012/09/19 21:59:34, scroggo-work wrote:
https://codereview.appspot.com/6531048/diff/1/src/utils/SkDeferredCanvas.cpp#...
> src/utils/SkDeferredCanvas.cpp:196: SkRefCnt_SafeAssign(fPlaybackDevice,
> canvas->getDevice());
> Since fPlaybackDevice is only used by ::nullPlayback(), I might prefer that we
> just use a local variable in that function. On the other hand, that would mean
> extra ref()s and unref()s.

Actually, I think we can get away with not having ref/unref.  We just have to
add a getter to the pipe reader to retrieve the playback canvas. I'll try that.

[junov1, 2012-09-20 19:57:12]

New Patch : using silent playback (newly added to SkGPipe) rather than playing
to a null device.

[Leon, 2012-09-20 20:00:29]

https://codereview.appspot.com/6531048/diff/9001/src/utils/SkDeferredCanvas.cpp
File src/utils/SkDeferredCanvas.cpp (left):

https://codereview.appspot.com/6531048/diff/9001/src/utils/SkDeferredCanvas.c...
src/utils/SkDeferredCanvas.cpp:407: this->endRecording();
I think you can now remove DeferredDevice::endRecording() and
DeferredPipeController::reset()

[reed1, 2012-09-20 20:01:06]

lgtm

https://codereview.appspot.com/6531048/diff/9001/src/utils/SkDeferredCanvas.cpp
File src/utils/SkDeferredCanvas.cpp (right):

https://codereview.appspot.com/6531048/diff/9001/src/utils/SkDeferredCanvas.c...
src/utils/SkDeferredCanvas.cpp:391: fFreshFrame = true;
a bool seems so inexplicable here (true i do want to flush?). I guess I always
hate bools :)

[junov1, 2012-09-20 20:33:13]

Patch. 

FYI, this patch justifiably regresses the performance of the
deferred_canvas_record bench (see patch description)

[Leon, 2012-09-20 20:41:38]

On 2012/09/20 20:33:13, junov1 wrote:
> Patch. 
> 
> FYI, this patch justifiably regresses the performance of the
> deferred_canvas_record bench (see patch description)

lgtm