Revert of Add alternate SkPicture::clone (https://codereview.chromium.org/381193002/)
authorrobertphillips <robertphillips@google.com>
Fri, 11 Jul 2014 13:45:58 +0000 (06:45 -0700)
committerCommit bot <commit-bot@chromium.org>
Fri, 11 Jul 2014 13:45:58 +0000 (06:45 -0700)
Reason for revert:
Going to try to just remove the many-at-once clone interface

Original issue's description:
> Add alternate SkPicture::clone
>
> This adds an alternate version of SkPicture::clone for two reasons:
>
> 1) Chromium uses the SkPicture copy constructor to unpack the pictures from the old-style clone interface (and I would like to remove the copy ctor)
>
> 2) This is part of the long term plan to wean Chrome off of cloning. Once pictures are thread safe we will switch the new SkPicture::clone call to just return 'this'. From there it is a small step to removing clone entirely.
>
> Note that the two versions of clone() is temporary. Once this is landed (and rolled) I will land a Chrome-side patch to remove their use of the old interface (Use new SkPicture::clone interface - https://codereview.chromium.org/380323002/)
>
> Committed: https://skia.googlesource.com/skia/+/e372e78223a8ce916d276d6e0420d552fb0267e9

R=mtklein@google.com, reed@google.com
TBR=mtklein@google.com, reed@google.com
NOTREECHECKS=true
NOTRY=true

Author: robertphillips@google.com

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

include/core/SkPicture.h
src/core/SkPicture.cpp

index 1bf3171..ca26eb4 100644 (file)
@@ -123,7 +123,6 @@ public:
      * SkPictures.
      */
     void clone(SkPicture* pictures, int count) const;
-    void clone(SkPicture* pictures[], int count) const;
 #endif
 
     /** Replays the drawing commands on the specified canvas.
index 03a18ab..9261664 100644 (file)
@@ -191,8 +191,8 @@ SkPicture::~SkPicture() {}
 #ifdef SK_SUPPORT_LEGACY_PICTURE_CLONE
 // fRecord TODO, fix by deleting this method
 SkPicture* SkPicture::clone() const {
-    SkPicture* clonedPicture;
-    this->clone(&clonedPicture, 1);
+    SkPicture* clonedPicture = SkNEW(SkPicture);
+    this->clone(clonedPicture, 1);
     return clonedPicture;
 }
 
@@ -261,72 +261,6 @@ void SkPicture::clone(SkPicture* pictures, int count) const {
         }
     }
 }
-
-// fRecord TODO, fix by deleting this method
-void SkPicture::clone(SkPicture* pictures[], int count) const {
-    SkPictCopyInfo copyInfo;
-
-    for (int i = 0; i < count; i++) {
-        SkPicture* clone = pictures[i] = SkNEW(SkPicture);
-
-        clone->needsNewGenID();
-        clone->fWidth = fWidth;
-        clone->fHeight = fHeight;
-        clone->fData.reset(NULL);
-        clone->fRecordWillPlayBackBitmaps = fRecordWillPlayBackBitmaps;
-
-        /*  We want to copy the src's playback. However, if that hasn't been built
-            yet, we need to fake a call to endRecording() without actually calling
-            it (since it is destructive, and we don't want to change src).
-         */
-        if (fData.get()) {
-            if (!copyInfo.initialized) {
-                int paintCount = SafeCount(fData->fPaints);
-
-                /* The alternative to doing this is to have a clone method on the paint and have it
-                 * make the deep copy of its internal structures as needed. The holdup to doing
-                 * that is at this point we would need to pass the SkBitmapHeap so that we don't
-                 * unnecessarily flatten the pixels in a bitmap shader.
-                 */
-                copyInfo.paintData.setCount(paintCount);
-
-                /* Use an SkBitmapHeap to avoid flattening bitmaps in shaders. If there already is
-                 * one, use it. If this SkPictureData was created from a stream, fBitmapHeap
-                 * will be NULL, so create a new one.
-                 */
-                if (fData->fBitmapHeap.get() == NULL) {
-                    // FIXME: Put this on the stack inside SkPicture::clone.
-                    SkBitmapHeap* heap = SkNEW(SkBitmapHeap);
-                    copyInfo.controller.setBitmapStorage(heap);
-                    heap->unref();
-                } else {
-                    copyInfo.controller.setBitmapStorage(fData->fBitmapHeap);
-                }
-
-                SkDEBUGCODE(int heapSize = SafeCount(fData->fBitmapHeap.get());)
-                for (int i = 0; i < paintCount; i++) {
-                    if (NeedsDeepCopy(fData->fPaints->at(i))) {
-                        copyInfo.paintData[i] =
-                            SkFlatData::Create<SkPaint::FlatteningTraits>(&copyInfo.controller,
-                                                              fData->fPaints->at(i), 0);
-
-                    } else {
-                        // this is our sentinel, which we use in the unflatten loop
-                        copyInfo.paintData[i] = NULL;
-                    }
-                }
-                SkASSERT(SafeCount(fData->fBitmapHeap.get()) == heapSize);
-
-                // needed to create typeface playback
-                copyInfo.controller.setupPlaybacks();
-                copyInfo.initialized = true;
-            }
-
-            clone->fData.reset(SkNEW_ARGS(SkPictureData, (*fData, &copyInfo)));
-            clone->fUniqueID = this->uniqueID(); // need to call method to ensure != 0
-        }
-    }
-}
 #endif//SK_SUPPORT_LEGACY_PICTURE_CLONE
 
 // fRecord OK