Remove SkPicture copy constructor

Given where we're heading with SkPicture why would you need to make a copy?

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

Author: robertphillips@google.com

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

diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h
index 03d983f..362f806 100644 (file)
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -64,10 +64,6 @@ public:
     };
 
     SkPicture();
-    /** Make a copy of the contents of src. If src records more drawing after
-        this call, those elements will not appear in this picture.
-    */
-    SkPicture(const SkPicture& src);
 
     /**  PRIVATE / EXPERIMENTAL -- do not call */
     void EXPERIMENTAL_addAccelData(const AccelData*) const;
@@ -178,7 +174,6 @@ public:
     /**
      * Returns true if any bitmaps may be produced when this SkPicture
      * is replayed.
-     * Returns false if called while still recording.
      */
     bool willPlayBackBitmaps() const;
 
@@ -255,9 +250,6 @@ private:
 
     SkPicture(int width, int height, const SkPictureRecord& record, bool deepCopyOps);
 
-    static void WriteTagSize(SkWriteBuffer& buffer, uint32_t tag, size_t size);
-    static void WriteTagSize(SkWStream* stream, uint32_t tag, size_t size);
-
     // An OperationList encapsulates a set of operation offsets into the picture byte
     // stream along with the CTMs needed for those operation.
     class OperationList : ::SkNoncopyable {

diff --git a/samplecode/SampleApp.cpp b/samplecode/SampleApp.cpp
index 37377ae..8beb66a 100644 (file)
--- a/samplecode/SampleApp.cpp
+++ b/samplecode/SampleApp.cpp
@@ -1359,13 +1359,11 @@ void SampleWindow::afterChildren(SkCanvas* orig) {
     }
 
     if (kPicture_DeviceType == fDeviceType) {
-        SkAutoTUnref<SkPicture> picture(fRecorder.endRecording());
+        SkAutoTUnref<const SkPicture> picture(fRecorder.endRecording());
 
         if (true) {
-            SkPicture* pict = new SkPicture(*picture);
             this->installDrawFilter(orig);
-            orig->drawPicture(pict);
-            pict->unref();
+            orig->drawPicture(picture);
         } else if (true) {
             SkDynamicMemoryWStream ostream;
             picture->serialize(&ostream);

diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index 9261664..b96caf9 100644 (file)
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -109,24 +109,6 @@ const char* DrawTypeToString(DrawType drawType) {
 }
 #endif
 
-#ifdef SK_DEBUG_VALIDATE
-static void validateMatrix(const SkMatrix* matrix) {
-    SkScalar scaleX = matrix->getScaleX();
-    SkScalar scaleY = matrix->getScaleY();
-    SkScalar skewX = matrix->getSkewX();
-    SkScalar skewY = matrix->getSkewY();
-    SkScalar perspX = matrix->getPerspX();
-    SkScalar perspY = matrix->getPerspY();
-    if (scaleX != 0 && skewX != 0)
-        SkDebugf("scaleX != 0 && skewX != 0\n");
-    SkASSERT(scaleX == 0 || skewX == 0);
-    SkASSERT(scaleY == 0 || skewY == 0);
-    SkASSERT(perspX == 0);
-    SkASSERT(perspY == 0);
-}
-#endif
-
-
 ///////////////////////////////////////////////////////////////////////////////
 
 // fRecord OK
@@ -151,14 +133,6 @@ SkPicture::SkPicture(int width, int height,
     fData.reset(SkNEW_ARGS(SkPictureData, (record, info, deepCopyOps)));
 }
 
-// The simplest / safest way to copy an SkRecord is to replay it into a new one.
-static SkRecord* copy(const SkRecord& src, int width, int height) {
-    SkRecord* dst = SkNEW(SkRecord);
-    SkRecorder recorder(dst, width, height);
-    SkRecordDraw(src, &recorder);
-    return dst;
-}
-
 // Create an SkPictureData-backed SkPicture from an SkRecord.
 // This for compatibility with serialization code only.  This is not cheap.
 static SkPicture* backport(const SkRecord& src, int width, int height) {
@@ -168,24 +142,6 @@ static SkPicture* backport(const SkRecord& src, int width, int height) {
 }
 
 // fRecord OK
-SkPicture::SkPicture(const SkPicture& src) : INHERITED() {
-    this->needsNewGenID();
-    fWidth = src.fWidth;
-    fHeight = src.fHeight;
-    fRecordWillPlayBackBitmaps = src.fRecordWillPlayBackBitmaps;
-
-    if (NULL != src.fData.get()) {
-        fData.reset(SkNEW_ARGS(SkPictureData, (*src.fData)));
-        fUniqueID = src.uniqueID();  // need to call method to ensure != 0
-    }
-
-    if (NULL != src.fRecord.get()) {
-        fRecord.reset(copy(*src.fRecord, fWidth, fHeight));
-        fUniqueID = src.uniqueID();  // need to call method to ensure != 0
-    }
-}
-
-// fRecord OK
 SkPicture::~SkPicture() {}
 
 #ifdef SK_SUPPORT_LEGACY_PICTURE_CLONE

diff --git a/src/core/SkPictureData.cpp b/src/core/SkPictureData.cpp
index e89ef37..746250d 100644 (file)
--- a/src/core/SkPictureData.cpp
+++ b/src/core/SkPictureData.cpp
@@ -191,31 +191,6 @@ SkPictureData::SkPictureData(const SkPictureData& src, SkPictCopyInfo* deepCopyI
         }
     }
 }
-#else
-SkPictureData::SkPictureData(const SkPictureData& src) : fInfo(src.fInfo) {
-    this->init();
-
-    fBitmapHeap.reset(SkSafeRef(src.fBitmapHeap.get()));
-    fPathHeap.reset(SkSafeRef(src.fPathHeap.get()));
-
-    fOpData = SkSafeRef(src.fOpData);
-
-    fBoundingHierarchy = src.fBoundingHierarchy;
-    fStateTree = src.fStateTree;
-    fContentInfo.set(src.fContentInfo);
-
-    SkSafeRef(fBoundingHierarchy);
-    SkSafeRef(fStateTree);
-
-    fBitmaps = SkSafeRef(src.fBitmaps);
-    fPaints = SkSafeRef(src.fPaints);
-
-    fPictureCount = src.fPictureCount;
-    fPictureRefs = SkNEW_ARRAY(const SkPicture*, fPictureCount);
-    for (int i = 0; i < fPictureCount; i++) {
-        fPictureRefs[i] = SkRef(src.fPictureRefs[i]);
-    }
-}
 #endif//SK_SUPPORT_LEGACY_PICTURE_CLONE
 
 void SkPictureData::init() {

diff --git a/src/core/SkPictureData.h b/src/core/SkPictureData.h
index 3f7ab00..15ea37b 100644 (file)
--- a/src/core/SkPictureData.h
+++ b/src/core/SkPictureData.h
@@ -132,8 +132,6 @@ class SkPictureData {
 public:
 #ifdef SK_SUPPORT_LEGACY_PICTURE_CLONE
     SkPictureData(const SkPictureData& src, SkPictCopyInfo* deepCopyInfo = NULL);
-#else
-    SkPictureData(const SkPictureData& src);
 #endif
     SkPictureData(const SkPictureRecord& record, const SkPictInfo&, bool deepCopyOps);
     static SkPictureData* CreateFromStream(SkStream*,

diff --git a/tests/PictureTest.cpp b/tests/PictureTest.cpp
index 61f6d03..a4dc7d7 100644 (file)
--- a/tests/PictureTest.cpp
+++ b/tests/PictureTest.cpp
@@ -1536,13 +1536,6 @@ static void test_gen_id(skiatest::Reporter* reporter) {
 
     // both pictures should have different ids
     REPORTER_ASSERT(reporter, hasData->uniqueID() != empty.uniqueID());
-
-    // test out copy constructor
-    SkPicture copyWithData(*hasData);
-    REPORTER_ASSERT(reporter, hasData->uniqueID() == copyWithData.uniqueID());
-
-    SkPicture emptyCopy(empty);
-    REPORTER_ASSERT(reporter, empty.uniqueID() != emptyCopy.uniqueID());
 }
 
 DEF_TEST(Picture, reporter) {