SkData -> SkPicture::SnapshotArray

Restores type safety with all the same features.

(Also note, less code: 29 insertions, 50 deletions.)

BUG=skia:

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

diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h
index e9500b2..ea3db8e 100644 (file)
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -199,6 +199,18 @@ public:
      */
     bool hasText() const;
 
+    // A refcounted array of refcounted const SkPicture pointers.
+    struct SnapshotArray : public SkNVRefCnt {
+        SnapshotArray(const SkPicture* pics[], size_t count) : fPics(pics), fCount(count) {}
+        ~SnapshotArray() { for (size_t i = 0; i < fCount; i++) { fPics[i]->unref(); } }
+
+        const SkPicture* const* begin() const { return fPics; }
+        size_t count() const { return fCount; }
+    private:
+        SkAutoTMalloc<const SkPicture*> fPics;
+        size_t fCount;
+    };
+
 private:
     // V2 : adds SkPixelRef's generation ID.
     // V3 : PictInfo tag at beginning, and EOF tag at the end
@@ -251,9 +263,8 @@ private:
     void createHeader(SkPictInfo* info) const;
     static bool IsValidPictInfo(const SkPictInfo& info);
 
-    // Takes ownership of the SkRecord, refs the (optional) drawablePicts and BBH.
-    SkPicture(const SkRect& cullRect, SkRecord*, SkData* drawablePicts,
-              SkBBoxHierarchy*);
+    // Takes ownership of the SkRecord, refs the (optional) SnapshotArray and BBH.
+    SkPicture(const SkRect& cullRect, SkRecord*, SnapshotArray*, SkBBoxHierarchy*);
 
     static SkPicture* Forwardport(const SkPictInfo&, const SkPictureData*);
     static SkPictureData* Backport(const SkRecord&, const SkPictInfo&,
@@ -266,7 +277,7 @@ private:
     mutable SkTDArray<DeletionListener*> fDeletionListeners;  // pointers are refed
     SkAutoTDelete<const SkRecord>       fRecord;
     SkAutoTUnref<const SkBBoxHierarchy> fBBH;
-    SkAutoTUnref<SkData>          fDrawablePicts;
+    SkAutoTUnref<const SnapshotArray>   fDrawablePicts;
 
     // helpers for fDrawablePicts
     int drawableCount() const;

diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index 4848345..a8e160f 100644 (file)
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -270,18 +270,11 @@ bool SkPicture::Analysis::suitableForGpuRasterization(const char** reason,
 ///////////////////////////////////////////////////////////////////////////////
 
 int SkPicture::drawableCount() const {
-    if (fDrawablePicts.get()) {
-        return SkToInt(fDrawablePicts->size() / sizeof(SkPicture*));
-    } else {
-        return 0;
-    }
+    return fDrawablePicts.get() ? fDrawablePicts->count() : 0;
 }
 
 SkPicture const* const* SkPicture::drawablePicts() const {
-    if (fDrawablePicts) {
-        return reinterpret_cast<SkPicture* const*>(fDrawablePicts->data());
-    }
-    return NULL;
+    return fDrawablePicts.get() ? fDrawablePicts->begin() : NULL;
 }
 
 SkPicture::~SkPicture() {
@@ -524,7 +517,7 @@ bool SkPicture::hasText()             const { return fAnalysis.fHasText; }
 bool SkPicture::willPlayBackBitmaps() const { return fAnalysis.fWillPlaybackBitmaps; }
 int  SkPicture::approximateOpCount()  const { return fRecord->count(); }
 
-SkPicture::SkPicture(const SkRect& cullRect, SkRecord* record, SkData* drawablePicts,
+SkPicture::SkPicture(const SkRect& cullRect, SkRecord* record, SnapshotArray* drawablePicts,
                      SkBBoxHierarchy* bbh)
     : fUniqueID(next_picture_generation_id())
     , fCullRect(cullRect)

diff --git a/src/core/SkPictureRecorder.cpp b/src/core/SkPictureRecorder.cpp
index aea9e38..edee07b 100644 (file)
--- a/src/core/SkPictureRecorder.cpp
+++ b/src/core/SkPictureRecorder.cpp
@@ -61,9 +61,10 @@ SkPicture* SkPictureRecorder::endRecording() {
     // TODO: we should remember these from our caller
     SkBBHFactory* factory = NULL;
     uint32_t recordFlags = 0;
-    SkAutoDataUnref drawablePicts(fRecorder->newDrawableSnapshot(factory, recordFlags));
+    SkAutoTUnref<SkPicture::SnapshotArray> drawablePicts(
+            fRecorder->newDrawableSnapshot(factory, recordFlags));
     SkPicture* pict = SkNEW_ARGS(SkPicture, (fCullRect, fRecord.detach(),
-                                             drawablePicts, fBBH.get()));
+                                             drawablePicts.detach(), fBBH.get()));
 
     if (saveLayerData) {
         pict->EXPERIMENTAL_addAccelData(saveLayerData);

diff --git a/src/core/SkRecorder.cpp b/src/core/SkRecorder.cpp
index 3252e59..22d742f 100644 (file)
--- a/src/core/SkRecorder.cpp
+++ b/src/core/SkRecorder.cpp
@@ -5,7 +5,6 @@
  * found in the LICENSE file.
  */
 
-#include "SkData.h"
 #include "SkRecorder.h"
 #include "SkPatchUtils.h"
 #include "SkPicture.h"
@@ -30,42 +29,17 @@ void SkRecorder::forgetRecord() {
     fRecord = NULL;
 }
 
-// ReleaseProc for SkData, assuming the data was allocated via sk_malloc, and its contents are an
-// array of SkPicture* which need to be unref'd.
-//
-static void unref_all_malloc_releaseProc(const void* ptr, size_t length, void* context) {
-    SkASSERT(ptr == context);   // our context is our ptr, allocated via sk_malloc
-    int count = SkToInt(length / sizeof(SkPicture*));
-    SkASSERT(count * sizeof(SkPicture*) == length);  // our length is snug for the array
-
-    SkPicture* const* array = reinterpret_cast<SkPicture* const*>(ptr);
-    for (int i = 0; i < count; ++i) {
-        SkSafeUnref(array[i]);
-    }
-    sk_free(context);
-}
-
-// Return an uninitialized SkData sized for "count" SkPicture pointers. They will be unref'd when
-// the SkData is destroyed.
-//
-static SkData* new_uninitialized_picture_ptrs(int count) {
-    size_t length = count * sizeof(SkPicture*);
-    void* array = sk_malloc_throw(length);
-    void* context = array;
-    return SkData::NewWithProc(array, length, unref_all_malloc_releaseProc, context);
-}
-
-SkData* SkRecorder::newDrawableSnapshot(SkBBHFactory* factory, uint32_t recordFlags) {
+SkPicture::SnapshotArray* SkRecorder::newDrawableSnapshot(SkBBHFactory* factory,
+                                                          uint32_t recordFlags) {
     const int count = fDrawableList.count();
     if (0 == count) {
         return NULL;
     }
-    SkData* data = new_uninitialized_picture_ptrs(count);
-    SkPicture** pics = reinterpret_cast<SkPicture**>(data->writable_data());
+    SkAutoTMalloc<const SkPicture*> pics(count);
     for (int i = 0; i < count; ++i) {
         pics[i] = fDrawableList[i]->newPictureSnapshot(factory, recordFlags);
     }
-    return data;
+    return SkNEW_ARGS(SkPicture::SnapshotArray, (pics.detach(), count));
 }
 
 // To make appending to fRecord a little less verbose.

diff --git a/src/core/SkRecorder.h b/src/core/SkRecorder.h
index 7b2cbfe..b532c89 100644 (file)
--- a/src/core/SkRecorder.h
+++ b/src/core/SkRecorder.h
@@ -22,9 +22,8 @@ public:
     SkRecorder(SkRecord*, const SkRect& bounds);
     virtual ~SkRecorder() SK_OVERRIDE;
 
-    // return a (new or ref'd) data containing the array of pictures that were
-    // snapped from our drawables.
-    SkData* newDrawableSnapshot(SkBBHFactory*, uint32_t recordFlags);
+    // Return a new or ref'd array of pictures that were snapped from our drawables.
+    SkPicture::SnapshotArray* newDrawableSnapshot(SkBBHFactory*, uint32_t recordFlags);
 
     // Make SkRecorder forget entirely about its SkRecord*; all calls to SkRecorder will fail.
     void forgetRecord();