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.

R=bsalomon@google.com, reed@google.com

Author: robertphillips@google.com

Review URL: https://codereview.chromium.org/267293007

git-svn-id: http://skia.googlecode.com/svn/trunk@14650 2bbb7eff-a529-9590-31e7-b0007b416f81

4 files changed, 28 insertions, 58 deletions

diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index 28ca689d01..f2356bdeff 100644
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -896,15 +896,15 @@ void SkPicturePlayback::draw(SkCanvas& canvas, SkDrawPictureCallback* callback)
             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
@@ -912,17 +912,32 @@ void SkPicturePlayback::draw(SkCanvas& canvas, SkDrawPictureCallback* callback)
                     // 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();
-                    }
 
-                    if (kDrawComplete == it.peekDraw()) {
+                    uint32_t skipTo;
+                    do {
+                        skipTo = it.nextDraw();
+                        if (kDrawComplete == skipTo) {
+                            break;
+                        }
+
+                        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);
diff --git a/src/core/SkPicturePlayback.h b/src/core/SkPicturePlayback.h
index 37caa8a43d..d6f0cf1919 100644
--- a/src/core/SkPicturePlayback.h
+++ b/src/core/SkPicturePlayback.h
@@ -273,7 +273,7 @@ private:
         struct ReplacementInfo {
             size_t          fStart;
             size_t          fStop;
-            SkPoint         fPos;
+            SkIPoint        fPos;
             SkBitmap*       fBM;
             const SkPaint*  fPaint;  // Note: this object doesn't own the paint
         };
@@ -293,14 +293,13 @@ private:
 
         void freeAll();
 
-    #ifdef SK_DEBUG
+#ifdef SK_DEBUG
         void validate() const;
-    #endif
+#endif
 
         SkTDArray<ReplacementInfo> fReplacements;
     };
 
-
     // Replace all the draw ops in the replacement ranges in 'replacements' with
     // the associated drawBitmap call
     // Draw replacing cannot be enabled at the same time as draw limiting
diff --git a/src/core/SkPictureStateTree.cpp b/src/core/SkPictureStateTree.cpp
index 8b5bc0aa2f..fdd86464a9 100644
--- a/src/core/SkPictureStateTree.cpp
+++ b/src/core/SkPictureStateTree.cpp
@@ -114,35 +114,6 @@ void SkPictureStateTree::Iterator::setCurrentMatrix(const SkMatrix* matrix) {
     fCurrentMatrix = matrix;
 }
 
-uint32_t SkPictureStateTree::Iterator::peekDraw() {
-    SkASSERT(this->isValid());
-    if (fPlaybackIndex >= fDraws->count()) {
-        return kDrawComplete;
-    }
-
-    Draw* draw = static_cast<Draw*>((*fDraws)[fPlaybackIndex]);
-    return draw->fOffset;
-}
-
-uint32_t SkPictureStateTree::Iterator::skipDraw() {
-    SkASSERT(this->isValid());
-    if (fPlaybackIndex >= fDraws->count()) {
-        return this->finish();
-    }
-
-    Draw* draw = static_cast<Draw*>((*fDraws)[fPlaybackIndex]);
-
-    if (fSave) {
-        fCanvas->save();
-        fSave = false;
-    }
-
-    fNodes.rewind();
-
-    ++fPlaybackIndex;
-    return draw->fOffset;
-}
-
 uint32_t SkPictureStateTree::Iterator::finish() {
     if (fCurrentNode->fFlags & Node::kSaveLayer_Flag) {
         fCanvas->restore();
diff --git a/src/core/SkPictureStateTree.h b/src/core/SkPictureStateTree.h
index f9e187a0cb..da51a5b954 100644
--- a/src/core/SkPictureStateTree.h
+++ b/src/core/SkPictureStateTree.h
@@ -82,21 +82,6 @@ public:
             TODO: this might be better named nextOp
         */
         uint32_t nextDraw();
-        /** Peek at the currently queued up draw op's offset. Note that this can
-            be different then what 'nextDraw' would return b.c. it is
-            the offset of the next _draw_ op while 'nextDraw' can return
-            the offsets to saveLayer and clip ops while it is creating the proper
-            drawing context for the queued up draw op.
-        */
-        uint32_t peekDraw();
-        /** Stop trying to create the drawing context for the currently queued
-            up _draw_ operation and queue up the next one. This call returns
-            the offset of the skipped _draw_ operation. Obviously (since the
-            correct drawing context has not been established), the skipped
-            _draw_ operation should not be issued. Returns kDrawComplete if
-            the end of the draw operations is reached.
-         */
-        uint32_t skipDraw();
         static const uint32_t kDrawComplete = SK_MaxU32;
         Iterator() : fPlaybackMatrix(), fValid(false) { }
         bool isValid() const { return fValid; }