Add a release procedure to SkMallocPixelRef; remove SkDataPixelRef
authorhalcanary@google.com <halcanary@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Thu, 2 Jan 2014 17:29:28 +0000 (17:29 +0000)
committerhalcanary@google.com <halcanary@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Thu, 2 Jan 2014 17:29:28 +0000 (17:29 +0000)
This works in a way that is similar to SkData.

SkMallocPixelRef::NewWithProc

    Motivation: Chrome has a ETC1PixelRef which calls delete[] on the
    pixles on destruction.  There is no reason for them to almost
    duplicate our class, when we can provide them a more flexible
    class.  Example use:

      static void delete_uint8_proc(void* ptr, void*) {
        delete[] static_cast<uint8_t>(ptr);
      }
      SkPixelRef* new_delete_pixref(const SkImageInfo& info,
                                    SkColorTable* ctable) {
        size_t rb = info.minRowBytes();
        return SkMallocPixelRef::NewWithProc(
          info, rb, ctable,
          new uint8_t[info.getSafeSize(rb)],
          delete_uint8_proc, NULL);
      }

SkMallocPixelRef::NewWithData

Motivation:  This allows up to eliminate SkDataPixelRef.  We
    modified SkImage_Raster to use MallocPixelRef rather than
    SkDataPixlRef.

Also:  Unit tests in tests/MallocPixelRefTest.

BUG=
R=reed@google.com

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

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

gyp/core.gypi
gyp/tests.gyp
include/core/SkMallocPixelRef.h
src/core/SkMallocPixelRef.cpp
src/image/SkDataPixelRef.cpp [deleted file]
src/image/SkDataPixelRef.h [deleted file]
src/image/SkImage_Raster.cpp
tests/MallocPixelRefTest.cpp [new file with mode: 0644]

index d5c1153..d3eb4f3 100644 (file)
 
         '<(skia_src_path)/doc/SkDocument.cpp',
 
-        '<(skia_src_path)/image/SkDataPixelRef.cpp',
         '<(skia_src_path)/image/SkImage.cpp',
         '<(skia_src_path)/image/SkImagePriv.cpp',
         '<(skia_src_path)/image/SkImage_Codec.cpp',
index d6a1a7c..ec8ae43 100644 (file)
@@ -96,6 +96,7 @@
         '../tests/LListTest.cpp',
         '../tests/LayerDrawLooperTest.cpp',
         '../tests/MD5Test.cpp',
+        '../tests/MallocPixelRefTest.cpp',
         '../tests/MathTest.cpp',
         '../tests/MatrixTest.cpp',
         '../tests/Matrix44Test.cpp',
index a547ddb..272dc21 100644 (file)
@@ -45,11 +45,46 @@ public:
     static SkMallocPixelRef* NewAllocate(const SkImageInfo& info,
                                          size_t rowBytes, SkColorTable*);
 
+    /**
+     *  Return a new SkMallocPixelRef with the provided pixel storage,
+     *  rowBytes, and optional colortable. On destruction, ReleaseProc
+     *  will be called.
+     *
+     *  This pixelref will ref() the specified colortable (if not NULL).
+     *
+     *  Returns NULL on failure.
+     */
+    typedef void (*ReleaseProc)(void* addr, void* context);
+    static SkMallocPixelRef* NewWithProc(const SkImageInfo& info,
+                                         size_t rowBytes, SkColorTable*,
+                                         void* addr, ReleaseProc proc,
+                                         void* context);
+
+    /**
+     *  Return a new SkMallocPixelRef that will use the provided
+     *  SkData, rowBytes, and optional colortable as pixel storage.
+     *  The SkData will be ref()ed and on destruction of the PielRef,
+     *  the SkData will be unref()ed.
+     *
+     *  @param offset (in bytes) into the provided SkData that the
+     *         first pixel is located at.
+     *
+     *  This pixelref will ref() the specified colortable (if not NULL).
+     *
+     *  Returns NULL on failure.
+     */
+    static SkMallocPixelRef* NewWithData(const SkImageInfo& info,
+                                         size_t rowBytes,
+                                         SkColorTable* ctable,
+                                         SkData* data,
+                                         size_t offset = 0);
+
     void* getAddr() const { return fStorage; }
 
     SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkMallocPixelRef)
 
 protected:
+    // The ownPixels version of this constructor is deprecated.
     SkMallocPixelRef(const SkImageInfo&, void* addr, size_t rb, SkColorTable*,
                      bool ownPixels);
     SkMallocPixelRef(SkFlattenableReadBuffer& buffer);
@@ -64,7 +99,11 @@ private:
     void*           fStorage;
     SkColorTable*   fCTable;
     size_t          fRB;
-    const bool      fOwnPixels;
+    ReleaseProc     fReleaseProc;
+    void*           fReleaseProcContext;
+
+    SkMallocPixelRef(const SkImageInfo&, void* addr, size_t rb, SkColorTable*,
+                     ReleaseProc proc, void* context);
 
     typedef SkPixelRef INHERITED;
 };
index 08cd067..c3e605c 100644 (file)
@@ -9,6 +9,11 @@
 #include "SkBitmap.h"
 #include "SkFlattenableBuffers.h"
 
+// assumes ptr was allocated via sk_malloc
+static void sk_free_releaseproc(void* ptr, void*) {
+    sk_free(ptr);
+}
+
 static bool is_valid(const SkImageInfo& info, SkColorTable* ctable) {
     if (info.fWidth < 0 ||
         info.fHeight < 0 ||
@@ -39,7 +44,8 @@ SkMallocPixelRef* SkMallocPixelRef::NewDirect(const SkImageInfo& info,
     if (!is_valid(info, ctable)) {
         return NULL;
     }
-    return SkNEW_ARGS(SkMallocPixelRef, (info, addr, rowBytes, ctable, false));
+    return SkNEW_ARGS(SkMallocPixelRef,
+                      (info, addr, rowBytes, ctable, NULL, NULL));
 }
 
 SkMallocPixelRef* SkMallocPixelRef::NewAllocate(const SkImageInfo& info,
@@ -70,12 +76,59 @@ SkMallocPixelRef* SkMallocPixelRef::NewAllocate(const SkImageInfo& info,
     }
 
     size_t size = sk_64_asS32(bigSize);
+    SkASSERT(size >= info.getSafeSize(rowBytes));
     void* addr = sk_malloc_flags(size, 0);
     if (NULL == addr) {
         return NULL;
     }
 
-    return SkNEW_ARGS(SkMallocPixelRef, (info, addr, rowBytes, ctable, true));
+    return SkNEW_ARGS(SkMallocPixelRef,
+                      (info, addr, rowBytes, ctable,
+                       sk_free_releaseproc, NULL));
+}
+
+SkMallocPixelRef* SkMallocPixelRef::NewWithProc(const SkImageInfo& info,
+                                                size_t rowBytes,
+                                                SkColorTable* ctable,
+                                                void* addr,
+                                                SkMallocPixelRef::ReleaseProc proc,
+                                                void* context) {
+    if (!is_valid(info, ctable)) {
+        return NULL;
+    }
+    return SkNEW_ARGS(SkMallocPixelRef,
+                      (info, addr, rowBytes, ctable, proc, context));
+}
+
+static void sk_data_releaseproc(void*, void* dataPtr) {
+    (static_cast<SkData*>(dataPtr))->unref();
+}
+
+SkMallocPixelRef* SkMallocPixelRef::NewWithData(const SkImageInfo& info,
+                                                size_t rowBytes,
+                                                SkColorTable* ctable,
+                                                SkData* data,
+                                                size_t offset) {
+    SkASSERT(data != NULL);
+    SkASSERT(offset <= data->size());
+    if (!is_valid(info, ctable)) {
+        return NULL;
+    }
+    if ((rowBytes < info.minRowBytes())
+        || ((data->size() - offset) < info.getSafeSize(rowBytes))) {
+        return NULL;
+    }
+    data->ref();
+    const void* ptr = static_cast<const void*>(data->bytes() + offset);
+    SkMallocPixelRef* pr
+        = SkNEW_ARGS(SkMallocPixelRef,
+                     (info, const_cast<void*>(ptr), rowBytes, ctable,
+                      sk_data_releaseproc, static_cast<void*>(data)));
+    SkASSERT(pr != NULL);
+    // We rely on the immutability of the pixels to make the
+    // const_cast okay.
+    pr->setImmutable();
+    return pr;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -84,7 +137,31 @@ SkMallocPixelRef::SkMallocPixelRef(const SkImageInfo& info, void* storage,
                                    size_t rowBytes, SkColorTable* ctable,
                                    bool ownsPixels)
     : INHERITED(info)
-    , fOwnPixels(ownsPixels)
+    , fReleaseProc(ownsPixels ? sk_free_releaseproc : NULL)
+    , fReleaseProcContext(NULL) {
+    // This constructor is now DEPRICATED.
+    SkASSERT(is_valid(info, ctable));
+    SkASSERT(rowBytes >= info.minRowBytes());
+
+    if (kIndex_8_SkColorType != info.fColorType) {
+        ctable = NULL;
+    }
+
+    fStorage = storage;
+    fCTable = ctable;
+    fRB = rowBytes;
+    SkSafeRef(ctable);
+
+    this->setPreLocked(fStorage, fCTable);
+}
+
+SkMallocPixelRef::SkMallocPixelRef(const SkImageInfo& info, void* storage,
+                                   size_t rowBytes, SkColorTable* ctable,
+                                   SkMallocPixelRef::ReleaseProc proc,
+                                   void* context)
+    : INHERITED(info)
+    , fReleaseProc(proc)
+    , fReleaseProcContext(context)
 {
     SkASSERT(is_valid(info, ctable));
     SkASSERT(rowBytes >= info.minRowBytes());
@@ -101,10 +178,11 @@ SkMallocPixelRef::SkMallocPixelRef(const SkImageInfo& info, void* storage,
     this->setPreLocked(fStorage, fCTable);
 }
 
+
 SkMallocPixelRef::~SkMallocPixelRef() {
     SkSafeUnref(fCTable);
-    if (fOwnPixels) {
-        sk_free(fStorage);
+    if (fReleaseProc != NULL) {
+        fReleaseProc(fStorage, fReleaseProcContext);
     }
 }
 
@@ -138,7 +216,8 @@ void SkMallocPixelRef::flatten(SkFlattenableWriteBuffer& buffer) const {
 
 SkMallocPixelRef::SkMallocPixelRef(SkFlattenableReadBuffer& buffer)
     : INHERITED(buffer, NULL)
-    , fOwnPixels(true)
+    , fReleaseProc(sk_free_releaseproc)
+    , fReleaseProcContext(NULL)
 {
     fRB = buffer.read32();
     size_t size = buffer.isValid() ? this->info().getSafeSize(fRB) : 0;
diff --git a/src/image/SkDataPixelRef.cpp b/src/image/SkDataPixelRef.cpp
deleted file mode 100644 (file)
index c8bfe80..0000000
+++ /dev/null
@@ -1,46 +0,0 @@
-/*
- * Copyright 2012 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#include "SkDataPixelRef.h"
-#include "SkData.h"
-#include "SkFlattenableBuffers.h"
-
-SkDataPixelRef::SkDataPixelRef(const SkImageInfo& info, SkData* data)
-    : INHERITED(info)
-    , fData(data)
-{
-    fData->ref();
-    this->setPreLocked(const_cast<void*>(fData->data()), NULL);
-}
-
-SkDataPixelRef::~SkDataPixelRef() {
-    fData->unref();
-}
-
-void* SkDataPixelRef::onLockPixels(SkColorTable** ct) {
-    *ct = NULL;
-    return const_cast<void*>(fData->data());
-}
-
-void SkDataPixelRef::onUnlockPixels() {
-    // nothing to do
-}
-
-size_t SkDataPixelRef::getAllocatedSizeInBytes() const {
-    return fData ? fData->size() : 0;
-}
-
-void SkDataPixelRef::flatten(SkFlattenableWriteBuffer& buffer) const {
-    this->INHERITED::flatten(buffer);
-    buffer.writeDataAsByteArray(fData);
-}
-
-SkDataPixelRef::SkDataPixelRef(SkFlattenableReadBuffer& buffer)
-        : INHERITED(buffer, NULL) {
-    fData = buffer.readByteArrayAsData();
-    this->setPreLocked(const_cast<void*>(fData->data()), NULL);
-}
diff --git a/src/image/SkDataPixelRef.h b/src/image/SkDataPixelRef.h
deleted file mode 100644 (file)
index 0f8269c..0000000
+++ /dev/null
@@ -1,36 +0,0 @@
-/*
- * Copyright 2012 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#ifndef SkDataPixelRef_DEFINED
-#define SkDataPixelRef_DEFINED
-
-#include "SkPixelRef.h"
-
-class SkData;
-
-class SkDataPixelRef : public SkPixelRef {
-public:
-            SkDataPixelRef(const SkImageInfo&, SkData* data);
-    virtual ~SkDataPixelRef();
-
-    SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkDataPixelRef)
-
-protected:
-    virtual void* onLockPixels(SkColorTable**) SK_OVERRIDE;
-    virtual void onUnlockPixels() SK_OVERRIDE;
-    virtual size_t getAllocatedSizeInBytes() const SK_OVERRIDE;
-
-    SkDataPixelRef(SkFlattenableReadBuffer& buffer);
-    virtual void flatten(SkFlattenableWriteBuffer&) const SK_OVERRIDE;
-
-private:
-    SkData* fData;
-
-    typedef SkPixelRef INHERITED;
-};
-
-#endif
index 807c19e..819c08f 100644 (file)
@@ -10,7 +10,7 @@
 #include "SkBitmap.h"
 #include "SkCanvas.h"
 #include "SkData.h"
-#include "SkDataPixelRef.h"
+#include "SkMallocPixelRef.h"
 
 class SkImage_Raster : public SkImage_Base {
 public:
@@ -85,7 +85,9 @@ SkImage* SkImage_Raster::NewEmpty() {
 SkImage_Raster::SkImage_Raster(const Info& info, SkData* data, size_t rowBytes)
         : INHERITED(info.fWidth, info.fHeight) {
     fBitmap.setConfig(info, rowBytes);
-    fBitmap.setPixelRef(SkNEW_ARGS(SkDataPixelRef, (info, data)))->unref();
+    SkAutoTUnref<SkPixelRef> ref(
+        SkMallocPixelRef::NewWithData(info, rowBytes, NULL, data, 0));
+    fBitmap.setPixelRef(ref, 0);
     fBitmap.setImmutable();
 }
 
diff --git a/tests/MallocPixelRefTest.cpp b/tests/MallocPixelRefTest.cpp
new file mode 100644 (file)
index 0000000..8fcdc86
--- /dev/null
@@ -0,0 +1,116 @@
+/*
+ * Copyright 2013 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkData.h"
+#include "SkMallocPixelRef.h"
+#include "Test.h"
+#include "TestClassDef.h"
+
+static void delete_uint8_proc(void* ptr, void*) {
+    delete[] static_cast<uint8_t*>(ptr);
+}
+
+static void set_to_one_proc(void*, void* context) {
+    *(static_cast<int*>(context)) = 1;
+}
+
+/**
+ *  This test contains basic sanity checks concerning SkMallocPixelRef.
+ */
+DEF_TEST(MallocPixelRef, reporter) {
+    REPORTER_ASSERT(reporter, true);
+    SkImageInfo info = {10, 13, kPMColor_SkColorType, kPremul_SkAlphaType};
+    {
+        SkAutoTUnref<SkMallocPixelRef> pr(
+            SkMallocPixelRef::NewAllocate(info, info.minRowBytes() - 1, NULL));
+        // rowbytes too small.
+        REPORTER_ASSERT(reporter, NULL == pr.get());
+    }
+    {
+        size_t rowBytes = info.minRowBytes() - 1;
+        size_t size = info.getSafeSize(rowBytes);
+        void* addr = sk_malloc_throw(size);
+        SkAutoDataUnref data(SkData::NewFromMalloc(addr, size));
+        SkAutoTUnref<SkMallocPixelRef> pr(
+            SkMallocPixelRef::NewWithData(info, rowBytes,
+                                          NULL, data.get()));
+        // rowbytes too small.
+        REPORTER_ASSERT(reporter, NULL == pr.get());
+    }
+    {
+        size_t rowBytes = info.minRowBytes() + 2;
+        size_t size = info.getSafeSize(rowBytes) - 1;
+        void* addr = sk_malloc_throw(size);
+        SkAutoDataUnref data(SkData::NewFromMalloc(addr, size));
+        SkAutoTUnref<SkMallocPixelRef> pr(
+            SkMallocPixelRef::NewWithData(info, rowBytes, NULL,
+                                          data.get()));
+        // data too small.
+        REPORTER_ASSERT(reporter, NULL == pr.get());
+    }
+    size_t rowBytes = info.minRowBytes() + 7;
+    size_t size = info.getSafeSize(rowBytes) + 9;
+    {
+        SkAutoMalloc memory(size);
+        SkAutoTUnref<SkMallocPixelRef> pr(
+            SkMallocPixelRef::NewDirect(info, memory.get(), rowBytes, NULL));
+        REPORTER_ASSERT(reporter, pr.get() != NULL);
+        REPORTER_ASSERT(reporter, memory.get() == pr->pixels());
+    }
+    {
+        SkAutoTUnref<SkMallocPixelRef> pr(
+            SkMallocPixelRef::NewAllocate(info, rowBytes, NULL));
+        REPORTER_ASSERT(reporter, pr.get() != NULL);
+        REPORTER_ASSERT(reporter, NULL != pr->pixels());
+    }
+    {
+        void* addr = static_cast<void*>(new uint8_t[size]);
+        SkAutoTUnref<SkMallocPixelRef> pr(
+            SkMallocPixelRef::NewWithProc(info, rowBytes, NULL, addr,
+                                          delete_uint8_proc, NULL));
+        REPORTER_ASSERT(reporter, pr.get() != NULL);
+        REPORTER_ASSERT(reporter, addr == pr->pixels());
+    }
+    {
+        int x = 0;
+        SkAutoMalloc memory(size);
+        REPORTER_ASSERT(reporter, memory.get() != NULL);
+        SkAutoTUnref<SkMallocPixelRef> pr(
+            SkMallocPixelRef::NewWithProc(info, rowBytes, NULL,
+                                          memory.get(), set_to_one_proc,
+                                          static_cast<void*>(&x)));
+        REPORTER_ASSERT(reporter, pr.get() != NULL);
+        REPORTER_ASSERT(reporter, memory.get() == pr->pixels());
+        REPORTER_ASSERT(reporter, 0 == x);
+        pr.reset(NULL);
+        // make sure that set_to_one_proc was called.
+        REPORTER_ASSERT(reporter, 1 == x);
+    }
+    {
+        void* addr = static_cast<void*>(new uint8_t[size]);
+        REPORTER_ASSERT(reporter, addr != NULL);
+        SkAutoTUnref<SkMallocPixelRef> pr(
+            SkMallocPixelRef::NewWithProc(info, rowBytes, NULL, addr,
+                                          delete_uint8_proc, NULL));
+        REPORTER_ASSERT(reporter, addr == pr->pixels());
+    }
+    {
+        void* addr = sk_malloc_throw(size);
+        SkAutoDataUnref data(SkData::NewFromMalloc(addr, size));
+        REPORTER_ASSERT(reporter, data.get() != NULL);
+        SkData* dataPtr = data.get();
+        REPORTER_ASSERT(reporter, dataPtr->unique());
+        SkAutoTUnref<SkMallocPixelRef> pr(
+            SkMallocPixelRef::NewWithData(info, rowBytes, NULL, data.get(), 4));
+        REPORTER_ASSERT(reporter, !(dataPtr->unique()));
+        data.reset(NULL);
+        REPORTER_ASSERT(reporter, dataPtr->unique());
+        REPORTER_ASSERT(reporter,
+            static_cast<const void*>(dataPtr->bytes() + 4) == pr->pixels());
+    }
+}
+