SkData can allocate room for its contents in the same block
authorreed <reed@google.com>
Thu, 11 Sep 2014 15:42:36 +0000 (08:42 -0700)
committerCommit bot <commit-bot@chromium.org>
Thu, 11 Sep 2014 15:42:36 +0000 (08:42 -0700)
BUG=skia:
R=bungeman@google.com, mtklein@google.com

Author: reed@google.com

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

include/core/SkData.h
src/core/SkBitmap.cpp
src/core/SkData.cpp
src/core/SkFlattenableSerialization.cpp
src/core/SkStream.cpp
src/images/SkDecodingImageGenerator.cpp
src/pdf/SkPDFFont.cpp
src/pdf/SkPDFGraphicState.cpp
src/pdf/SkPDFShader.cpp
src/sfnt/SkOTUtils.cpp
src/utils/SkTextureCompressor.cpp

index fba2846c7045fccfaf3faeaee056bfc3830df952..6da8e113663f0bf256cd516b7e6576be71cd1d41 100644 (file)
@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *
@@ -6,8 +5,6 @@
  * found in the LICENSE file.
  */
 
-
-
 #ifndef SkData_DEFINED
 #define SkData_DEFINED
 
@@ -44,6 +41,19 @@ public:
         return reinterpret_cast<const uint8_t*>(fPtr);
     }
 
+    /**
+     *  USE WITH CAUTION.
+     *  This call will assert that the refcnt is 1, as a precaution against modifying the
+     *  contents when another client/thread has access to the data.
+     */
+    void* writable_data() {
+        if (fSize) {
+            // only assert we're unique if we're not empty
+            SkASSERT(this->unique());
+        }
+        return fPtr;
+    }
+
     /**
      *  Helper to copy a range of the data into a caller-provided buffer.
      *  Returns the actual number of bytes copied, after clamping offset and
@@ -69,6 +79,12 @@ public:
      */
     static SkData* NewWithCopy(const void* data, size_t length);
 
+    /**
+     *  Create a new data with uninitialized contents. The caller should call writable_data()
+     *  to write into the buffer, but this must be done before another ref() is made.
+     */
+    static SkData* NewUninitialized(size_t length);
+
     /**
      *  Create a new dataref by copying the specified c-string
      *  (a null-terminated array of bytes). The returned SkData will have size()
@@ -81,8 +97,15 @@ public:
      *  Create a new dataref, taking the data ptr as is, and using the
      *  releaseproc to free it. The proc may be NULL.
      */
-    static SkData* NewWithProc(const void* data, size_t length,
-                               ReleaseProc proc, void* context);
+    static SkData* NewWithProc(const void* data, size_t length, ReleaseProc proc, void* context);
+
+    /**
+     *  Call this when the data parameter is already const and will outlive the lifetime of the
+     *  SkData. Suitable for with const globals.
+     */
+    static SkData* NewWithoutCopy(const void* data, size_t length) {
+        return NewWithProc(data, length, NULL, NULL);
+    }
 
     /**
      *  Create a new dataref from a pointer allocated by malloc. The Data object
@@ -130,16 +153,22 @@ private:
     ReleaseProc fReleaseProc;
     void*       fReleaseProcContext;
 
-    const void* fPtr;
+    void*       fPtr;
     size_t      fSize;
 
     SkData(const void* ptr, size_t size, ReleaseProc, void* context);
+    SkData(size_t size);   // inplace new/delete
     virtual ~SkData();
 
+    virtual void internal_dispose() const SK_OVERRIDE;
+
     // Called the first time someone calls NewEmpty to initialize the singleton.
     static SkData* NewEmptyImpl();
     static void DeleteEmpty(SkData*);
 
+    // shared internal factory
+    static SkData* PrivateNewWithCopy(const void* srcOrNull, size_t length);
+
     typedef SkRefCnt INHERITED;
 };
 
index e28c6fdc3488ff486d98a338828389d5d2e4f131..f0ad029bea9285b74f03bbb0700ad09d1255c797 100644 (file)
@@ -1211,9 +1211,9 @@ bool SkBitmap::ReadRawPixels(SkReadBuffer* buffer, SkBitmap* bitmap) {
         return false;
     }
 
-    char* dst = (char*)sk_malloc_throw(ramSize);
+    SkAutoDataUnref data(SkData::NewUninitialized(ramSize));
+    char* dst = (char*)data->writable_data();
     buffer->readByteArray(dst, snugSize);
-    SkAutoDataUnref data(SkData::NewFromMalloc(dst, ramSize));
 
     if (snugSize != ramSize) {
         const char* srcRow = dst + snugRB * (height - 1);
index c65328720a789051e49be9d41d4a0a6866a022fc..a7a3d8d559681bf661cb90e4bbaf9a144f26ded7 100644 (file)
 #include "SkReadBuffer.h"
 #include "SkWriteBuffer.h"
 
+static void sk_inplace_sentinel_releaseproc(const void*, size_t, void*) {
+    // we should never get called, as we are just a sentinel
+    sk_throw();
+}
+
 SkData::SkData(const void* ptr, size_t size, ReleaseProc proc, void* context) {
-    fPtr = ptr;
+    fPtr = const_cast<void*>(ptr);
     fSize = size;
     fReleaseProc = proc;
     fReleaseProcContext = context;
 }
 
+// This constructor means we are inline with our fPtr's contents. Thus we set fPtr
+// to point right after this. We also set our releaseproc to sk_inplace_sentinel_releaseproc,
+// since we need to handle "delete" ourselves. See internal_displose().
+//
+SkData::SkData(size_t size) {
+    fPtr = (char*)(this + 1);   // contents are immediately after this
+    fSize = size;
+    fReleaseProc = sk_inplace_sentinel_releaseproc;
+    fReleaseProcContext = NULL;
+}
+
 SkData::~SkData() {
     if (fReleaseProc) {
         fReleaseProc(fPtr, fSize, fReleaseProcContext);
     }
 }
 
+void SkData::internal_dispose() const {
+    if (sk_inplace_sentinel_releaseproc == fReleaseProc) {
+        const_cast<SkData*>(this)->fReleaseProc = NULL;    // so we don't call it in our destructor
+
+        this->internal_dispose_restore_refcnt_to_1();
+        this->~SkData();        // explicitly call this for refcnt bookkeeping
+
+        sk_free(const_cast<SkData*>(this));
+    } else {
+        this->internal_dispose_restore_refcnt_to_1();
+        SkDELETE(this);
+    }
+}
+
 bool SkData::equals(const SkData* other) const {
     if (NULL == other) {
         return false;
@@ -47,11 +77,24 @@ size_t SkData::copyRange(size_t offset, size_t length, void* buffer) const {
     return length;
 }
 
+SkData* SkData::PrivateNewWithCopy(const void* srcOrNull, size_t length) {
+    if (0 == length) {
+        return SkData::NewEmpty();
+    }
+    char* storage = (char*)sk_malloc_throw(sizeof(SkData) + length);
+    SkData* data = new (storage) SkData(length);
+    if (srcOrNull) {
+        memcpy(data->writable_data(), srcOrNull, length);
+    }
+    return data;
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
 SkData* SkData::NewEmptyImpl() {
     return new SkData(NULL, 0, NULL, NULL);
 }
+
 void SkData::DeleteEmpty(SkData* ptr) { SkDELETE(ptr); }
 
 SkData* SkData::NewEmpty() {
@@ -68,14 +111,13 @@ SkData* SkData::NewFromMalloc(const void* data, size_t length) {
     return new SkData(data, length, sk_free_releaseproc, NULL);
 }
 
-SkData* SkData::NewWithCopy(const void* data, size_t length) {
-    if (0 == length) {
-        return SkData::NewEmpty();
-    }
+SkData* SkData::NewWithCopy(const void* src, size_t length) {
+    SkASSERT(src);
+    return PrivateNewWithCopy(src, length);
+}
 
-    void* copy = sk_malloc_throw(length); // balanced in sk_free_releaseproc
-    memcpy(copy, data, length);
-    return new SkData(copy, length, sk_free_releaseproc, NULL);
+SkData* SkData::NewUninitialized(size_t length) {
+    return PrivateNewWithCopy(NULL, length);
 }
 
 SkData* SkData::NewWithProc(const void* data, size_t length,
index b33bca6dbbe47c9a0fb15ed71289abb1f943a035..31602079a39b995897d906c8996b726a92478b42 100644 (file)
@@ -15,9 +15,9 @@ SkData* SkValidatingSerializeFlattenable(SkFlattenable* flattenable) {
     SkWriteBuffer writer(SkWriteBuffer::kValidation_Flag);
     writer.writeFlattenable(flattenable);
     size_t size = writer.bytesWritten();
-    void* data = sk_malloc_throw(size);
-    writer.writeToMemory(data);
-    return SkData::NewFromMalloc(data, size);
+    SkData* data = SkData::NewUninitialized(size);
+    writer.writeToMemory(data->writable_data());
+    return data;
 }
 
 SkFlattenable* SkValidatingDeserializeFlattenable(const void* data, size_t size,
index ca9f51f0615def31531c4cd8d88becd116377e93..b04bc162d2b96221f2e98b37ead8f6746ef6419a 100644 (file)
@@ -69,9 +69,9 @@ SkData* SkStream::readData() {
     if (0 == size) {
         return SkData::NewEmpty();
     } else {
-        void* buffer = sk_malloc_throw(size);
-        this->read(buffer, size);
-        return SkData::NewFromMalloc(buffer, size);
+        SkData* data = SkData::NewUninitialized(size);
+        this->read(data->writable_data(), size);
+        return data;
     }
 }
 
@@ -308,7 +308,7 @@ static SkData* newFromParams(const void* src, size_t size, bool copyData) {
     if (copyData) {
         return SkData::NewWithCopy(src, size);
     } else {
-        return SkData::NewWithProc(src, size, NULL, NULL);
+        return SkData::NewWithoutCopy(src, size);
     }
 }
 
@@ -318,7 +318,7 @@ SkMemoryStream::SkMemoryStream() {
 }
 
 SkMemoryStream::SkMemoryStream(size_t size) {
-    fData = SkData::NewFromMalloc(sk_malloc_throw(size), size);
+    fData = SkData::NewUninitialized(size);
     fOffset = 0;
 }
 
@@ -654,12 +654,12 @@ void SkDynamicMemoryWStream::padToAlign4()
 
 SkData* SkDynamicMemoryWStream::copyToData() const {
     if (NULL == fCopy) {
-        void* buffer = sk_malloc_throw(fBytesWritten);
-        this->copyTo(buffer);
-        fCopy = SkData::NewFromMalloc(buffer, fBytesWritten);
+        SkData* data = SkData::NewUninitialized(fBytesWritten);
+        // be sure to call copyTo() before we assign to fCopy
+        this->copyTo(data->writable_data());
+        fCopy = data;
     }
-    fCopy->ref();
-    return fCopy;
+    return SkRef(fCopy);
 }
 
 void SkDynamicMemoryWStream::invalidateCopy() {
@@ -891,11 +891,12 @@ SkData* SkCopyStreamToData(SkStream* stream) {
 
     if (stream->hasLength()) {
         const size_t length = stream->getLength();
-        SkAutoMalloc dst(length);
-        if (stream->read(dst.get(), length) != length) {
-            return NULL;
+        SkData* data = SkData::NewUninitialized(length);
+        if (stream->read(data->writable_data(), length) != length) {
+            data->unref();
+            data = NULL;
         }
-        return SkData::NewFromMalloc(dst.detach(), length);
+        return data;
     }
 
     SkDynamicMemoryWStream tempStream;
@@ -922,11 +923,9 @@ SkStreamRewindable* SkStreamRewindableFromSkStream(SkStream* stream) {
         if (stream->hasPosition()) {  // If stream has length, but can't rewind.
             length -= stream->getPosition();
         }
-        SkAutoMalloc allocMemory(length);
-        SkDEBUGCODE(size_t read =) stream->read(allocMemory.get(), length);
+        SkAutoTUnref<SkData> data(SkData::NewUninitialized(length));
+        SkDEBUGCODE(size_t read =) stream->read(data->writable_data(), length);
         SkASSERT(length == read);
-        SkAutoTUnref<SkData> data(
-                SkData::NewFromMalloc(allocMemory.detach(), length));
         return SkNEW_ARGS(SkMemoryStream, (data.get()));
     }
     SkDynamicMemoryWStream tempStream;
index 1d91bcc0f98e359c95b59cac8ac73d79feacdecf..359eb370adc29c240f48338fc574fe33376c9017 100644 (file)
@@ -132,22 +132,20 @@ DecodingImageGenerator::~DecodingImageGenerator() {
 SkData* DecodingImageGenerator::onRefEncodedData() {
     // This functionality is used in `gm --serialize`
     // Does not encode options.
-    if (fData != NULL) {
-        return SkSafeRef(fData);
-    }
-    // TODO(halcanary): SkStreamRewindable needs a refData() function
-    // which returns a cheap copy of the underlying data.
-    if (!fStream->rewind()) {
-        return NULL;
-    }
-    size_t length = fStream->getLength();
-    if (0 == length) {
-        return NULL;
+    if (NULL == fData) {
+        // TODO(halcanary): SkStreamRewindable needs a refData() function
+        // which returns a cheap copy of the underlying data.
+        if (!fStream->rewind()) {
+            return NULL;
+        }
+        size_t length = fStream->getLength();
+        if (0 == length) {
+            return NULL;
+        }
+        fData = SkData::NewUninitialized(length);
+        SkCheckResult(fStream->read(fData->writable_data(), length), length);
     }
-    void* buffer = sk_malloc_flags(length, 0);
-    SkCheckResult(fStream->read(buffer, length), length);
-    fData = SkData::NewFromMalloc(buffer, length);
-    return SkSafeRef(fData);
+    return SkRef(fData);
 }
 
 bool DecodingImageGenerator::onGetPixels(const SkImageInfo& info,
index 9e13053c60db85de10d61ed01637c8a89fb08473..b4e6d98722ab61d029e324748c2c6a19d5117458 100644 (file)
@@ -204,16 +204,15 @@ static SkData* handle_type1_stream(SkStream* srcStream, size_t* headerLen,
         SkASSERT(length > 0);
         SkASSERT(length + (2 * kPFBSectionHeaderLength) <= srcLen);
 
-        SkAutoTMalloc<uint8_t> buffer(length);
+        SkData* data = SkData::NewUninitialized(length);
 
         const uint8_t* const srcHeader = src + kPFBSectionHeaderLength;
         // There is a six-byte section header before header and data
         // (but not trailer) that we're not going to copy.
-        const uint8_t* const srcData
-            = srcHeader + *headerLen + kPFBSectionHeaderLength;
+        const uint8_t* const srcData = srcHeader + *headerLen + kPFBSectionHeaderLength;
         const uint8_t* const srcTrailer = srcData + *headerLen;
 
-        uint8_t* const resultHeader = buffer.get();
+        uint8_t* const resultHeader = (uint8_t*)data->writable_data();
         uint8_t* const resultData = resultHeader + *headerLen;
         uint8_t* const resultTrailer = resultData + *dataLen;
 
@@ -223,7 +222,7 @@ static SkData* handle_type1_stream(SkStream* srcStream, size_t* headerLen,
         memcpy(resultData,    srcData,    *dataLen);
         memcpy(resultTrailer, srcTrailer, *trailerLen);
 
-        return SkData::NewFromMalloc(buffer.detach(), length);
+        return data;
     }
 
     // A PFA has to be converted for PDF.
index bd8afe0684365fe64059df1f69cb530cd96254c5..7664ed699b24f89a2385f3438610e9d9d6f1599a 100644 (file)
@@ -123,7 +123,7 @@ SkPDFObject* SkPDFGraphicState::GetInvertFunction() {
         static const char psInvert[] = "{1 exch sub}";
         // Do not copy the trailing '\0' into the SkData.
         SkAutoTUnref<SkData> psInvertStream(
-                SkData::NewWithCopy(psInvert, strlen(psInvert)));
+                SkData::NewWithoutCopy(psInvert, strlen(psInvert)));
 
         invertFunction = new SkPDFStream(psInvertStream.get());
         invertFunction->insertInt("FunctionType", 4);
index afe6bb06b33704139ca2cde0991a91c13e22c45b..d9b701c081048868ec228239a83d0d48e6918249 100644 (file)
@@ -1160,10 +1160,8 @@ SkPDFImageShader::SkPDFImageShader(SkPDFShader::State* state) : fState(state) {
     fState.get()->fImage.unlockPixels();
 }
 
-SkPDFStream* SkPDFFunctionShader::makePSFunction(const SkString& psCode,
-                                                 SkPDFArray* domain) {
-    SkAutoDataUnref funcData(SkData::NewWithCopy(psCode.c_str(),
-                                                 psCode.size()));
+SkPDFStream* SkPDFFunctionShader::makePSFunction(const SkString& psCode, SkPDFArray* domain) {
+    SkAutoDataUnref funcData(SkData::NewWithCopy(psCode.c_str(), psCode.size()));
     SkPDFStream* result = new SkPDFStream(funcData.get());
     result->insertInt("FunctionType", 4);
     result->insert("Domain", domain);
@@ -1171,14 +1169,12 @@ SkPDFStream* SkPDFFunctionShader::makePSFunction(const SkString& psCode,
     return result;
 }
 
-SkPDFShader::ShaderCanonicalEntry::ShaderCanonicalEntry(SkPDFObject* pdfShader,
-                                                        const State* state)
-    : fPDFShader(pdfShader),
-      fState(state) {
-}
+SkPDFShader::ShaderCanonicalEntry::ShaderCanonicalEntry(SkPDFObject* pdfShader, const State* state)
+    : fPDFShader(pdfShader)
+    , fState(state)
+{}
 
-bool SkPDFShader::ShaderCanonicalEntry::operator==(
-        const ShaderCanonicalEntry& b) const {
+bool SkPDFShader::ShaderCanonicalEntry::operator==(const ShaderCanonicalEntry& b) const {
     return fPDFShader == b.fPDFShader ||
            (fState != NULL && b.fState != NULL && *fState == *b.fState);
 }
index e76d1da08c428a24122fae12185ff9f46bd9ffb7..0e009528b4dcd1aa14f70a22f611cdd684ee3088 100644 (file)
@@ -83,8 +83,8 @@ SkData* SkOTUtils::RenameFont(SkStream* fontData, const char* fontName, int font
     size_t originalDataSize = fontData->getLength() - oldNameTablePhysicalSize;
     size_t newDataSize = originalDataSize + nameTablePhysicalSize;
 
-    SK_OT_BYTE* data = static_cast<SK_OT_BYTE*>(sk_malloc_throw(newDataSize));
-    SkAutoTUnref<SkData> rewrittenFontData(SkData::NewFromMalloc(data, newDataSize));
+    SkAutoTUnref<SkData> rewrittenFontData(SkData::NewUninitialized(newDataSize));
+    SK_OT_BYTE* data = static_cast<SK_OT_BYTE*>(rewrittenFontData->writable_data());
 
     if (fontData->read(data, oldNameTableOffset) < oldNameTableOffset) {
         return NULL;
index 07b58c5a607639f64df06fd2174d83a04b9cb97f..799eadc84b8e3df26ed16f4f4f49fad096a28af1 100644 (file)
@@ -173,7 +173,7 @@ bool CompressBufferToFormat(uint8_t* dst, const uint8_t* src, SkColorType srcCol
     return false;
 }
 
-SkData *CompressBitmapToFormat(const SkBitmap &bitmap, Format format) {
+SkDataCompressBitmapToFormat(const SkBitmap &bitmap, Format format) {
     SkAutoLockPixels alp(bitmap);
 
     int compressedDataSize = GetCompressedDataSize(format, bitmap.width(), bitmap.height());
@@ -182,15 +182,14 @@ SkData *CompressBitmapToFormat(const SkBitmap &bitmap, Format format) {
     }
 
     const uint8_t* src = reinterpret_cast<const uint8_t*>(bitmap.getPixels());
-    uint8_t* dst = reinterpret_cast<uint8_t*>(sk_malloc_throw(compressedDataSize));
+    SkData* dst = SkData::NewUninitialized(compressedDataSize);
 
-    if (CompressBufferToFormat(dst, src, bitmap.colorType(), bitmap.width(), bitmap.height(),
-                               bitmap.rowBytes(), format)) {
-        return SkData::NewFromMalloc(dst, compressedDataSize);
+    if (!CompressBufferToFormat((uint8_t*)dst->writable_data(), src, bitmap.colorType(),
+                                bitmap.width(), bitmap.height(), bitmap.rowBytes(), format)) {
+        dst->unref();
+        dst = NULL;
     }
-
-    sk_free(dst);
-    return NULL;
+    return dst;
 }
 
 SkBlitter* CreateBlitterForFormat(int width, int height, void* compressedBuffer,