Fix rendering artifacts in pull-saveLayers-forward mode

Fix rendering artifacts in pull-saveLayers-forward mode

This CL fixes 2 bugs in the pre-rendering of saveLayers:

1) The drawBitmapRect call wasn't occurring in device space so could sometimes be double transformed

2) The BBH op skipping in SkPicturePlayback could sometimes mess up the SkPictureStateTree's state

It also reduces the number of layers that are pre-rendered when a BBH is not in use.

Committed: http://code.google.com/p/skia/source/detail?r=14650

Committed: http://code.google.com/p/skia/source/detail?r=14659

[bsalomon, 2014-05-08 13:50:45]

lgtm

https://codereview.chromium.org/267293007/diff/60001/src/core/SkPicturePlayba...
File src/core/SkPicturePlayback.cpp (right):

https://codereview.chromium.org/267293007/diff/60001/src/core/SkPicturePlayba...
src/core/SkPicturePlayback.cpp:926: // Convert nested saveLayers into saves
?

[robertphillips, 2014-05-08 16:49:36]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2014-05-08 16:50:32]

CQ is trying da patch. Follow status at

https://skia-tree-status.appspot.com/cq/robertphillips@google.com/267293007/8...

[commit-bot: I haz the power, 2014-05-08 16:50:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-08 16:50:35]

Failed to apply patch for src/core/SkPicturePlayback.cpp:
While running patch -p0 --forward --force --no-backup-if-mismatch;
  patching file src/core/SkPicturePlayback.cpp
  Hunk #1 FAILED at 896.
  1 out of 1 hunk FAILED -- saving rejects to file
src/core/SkPicturePlayback.cpp.rej

Patch:       src/core/SkPicturePlayback.cpp
Index: src/core/SkPicturePlayback.cpp
===================================================================
--- src/core/SkPicturePlayback.cpp	(revision 14632)
+++ src/core/SkPicturePlayback.cpp	(working copy)
@@ -896,33 +896,47 @@
             if (NULL != temp) {
                 SkASSERT(NULL != temp->fBM);
                 SkASSERT(NULL != temp->fPaint);
+                canvas.save();
+                canvas.setMatrix(initialMatrix);
                 canvas.drawBitmap(*temp->fBM, temp->fPos.fX, temp->fPos.fY,
temp->fPaint);
+                canvas.restore();
 
                 if (it.isValid()) {
                     // This save is needed since the BBH will automatically
issue
                     // a restore to balanced the saveLayer we're skipping
                     canvas.save();
-                    // Note: This skipping only works if the client only issues
-                    // well behaved saveLayer calls (i.e., doesn't use 
-                    // kMatrix_SaveFlag or kClip_SaveFlag in isolation)
-
                     // At this point we know that the PictureStateTree was
aiming
                     // for some draw op within temp's saveLayer (although
potentially
                     // in a separate saveLayer nested inside it).
                     // We need to skip all the operations inside temp's range
                     // along with all the associated state changes but update 
                     // the state tree to the first operation outside temp's
range.
-                    SkASSERT(it.peekDraw() >= temp->fStart && it.peekDraw() <=
temp->fStop);
 
-                    while (kDrawComplete != it.peekDraw() && it.peekDraw() <=
temp->fStop) {
-                        it.skipDraw();
-                    }
+                    uint32_t skipTo;
+                    do {
+                        skipTo = it.nextDraw();
+                        if (kDrawComplete == skipTo) {
+                            break;
+                        }
 
-                    if (kDrawComplete == it.peekDraw()) {
+                        if (skipTo <= temp->fStop) {
+                            reader.setOffset(skipTo);
+                            uint32_t size;
+                            DrawType op = read_op_and_size(&reader, &size);
+                            // Since we are relying on the normal
SkPictureStateTree
+                            // playback we need to convert any nested saveLayer
calls
+                            // it may issue into saves (so that all its
internal
+                            // restores will be balanced).
+                            if (SAVE_LAYER == op) {
+                                canvas.save();
+                            }
+                        }
+                    } while (skipTo <= temp->fStop);
+
+                    if (kDrawComplete == skipTo) {
                         break;
                     }
 
-                    uint32_t skipTo = it.nextDraw();
                     reader.setOffset(skipTo);
                 } else {
                     reader.setOffset(temp->fStop);

[robertphillips, 2014-05-08 17:06:51]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2014-05-08 17:07:57]

CQ is trying da patch. Follow status at

https://skia-tree-status.appspot.com/cq/robertphillips@google.com/267293007/1...

[commit-bot: I haz the power, 2014-05-08 18:13:11]

Change committed as 14650

[robertphillips, 2014-05-08 18:45:41]

This was reverted in r14654 due to unit test failures.

https://codereview.chromium.org/267293007/diff/60001/src/core/SkPicturePlayba...
File src/core/SkPicturePlayback.cpp (right):

https://codereview.chromium.org/267293007/diff/60001/src/core/SkPicturePlayba...
src/core/SkPicturePlayback.cpp:926: // Convert nested saveLayers into saves
On 2014/05/08 13:50:46, bsalomon wrote:
> ?

Done - expanded comment.

[robertphillips, 2014-05-08 23:15:42]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2014-05-08 23:17:26]

CQ is trying da patch. Follow status at

https://skia-tree-status.appspot.com/cq/robertphillips@google.com/267293007/1...

[commit-bot: I haz the power, 2014-05-08 23:24:09]

Change committed as 14659