diff options
author | commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> | 2014-03-12 17:04:28 +0000 |
---|---|---|
committer | commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> | 2014-03-12 17:04:28 +0000 |
commit | c30dcb9b128887c7e16afe32fdf35105cc42380b (patch) | |
tree | 97f55d320d7191e0f876ff9dc3ad2d1528064e62 | |
parent | 84cd099704b3896ca66081a96508572a924f850c (diff) |
Add capture snapshot as data to SkWriter32, use it to optimise record->playback.
This is a new way of implementing https://codereview.chromium.org/155863005/
It uses copy on write semantics to return a buffer without copying it, so that record -> playback does not need to copy the buffer.
BUG=skia:2125
R=tomhudson@google.com, mtklein@google.com, reed@google.com, iancottrell@chromium.org
Author: iancottrell@google.com
Review URL: https://codereview.chromium.org/167113003
git-svn-id: http://skia.googlecode.com/svn/trunk@13769 2bbb7eff-a529-9590-31e7-b0007b416f81
-rw-r--r-- | include/core/SkWriter32.h | 20 | ||||
-rw-r--r-- | src/core/SkPicturePlayback.cpp | 10 | ||||
-rw-r--r-- | src/core/SkWriter32.cpp | 28 | ||||
-rw-r--r-- | tests/Writer32Test.cpp | 33 |
4 files changed, 81 insertions, 10 deletions
diff --git a/include/core/SkWriter32.h b/include/core/SkWriter32.h index 2e7c9563d9..cadcaa7853 100644 --- a/include/core/SkWriter32.h +++ b/include/core/SkWriter32.h @@ -10,6 +10,7 @@ #ifndef SkWriter32_DEFINED #define SkWriter32_DEFINED +#include "SkData.h" #include "SkMatrix.h" #include "SkPath.h" #include "SkPoint.h" @@ -44,6 +45,7 @@ public: SkASSERT(SkIsAlign4((uintptr_t)external)); SkASSERT(SkIsAlign4(externalBytes)); + fSnapshot.reset(NULL); fData = (uint8_t*)external; fCapacity = externalBytes; fUsed = 0; @@ -70,7 +72,7 @@ public: /** * Read a T record at offset, which must be a multiple of 4. Only legal if the record - * was writtern atomically using the write methods below. + * was written atomically using the write methods below. */ template<typename T> const T& readTAt(size_t offset) const { @@ -81,12 +83,13 @@ public: /** * Overwrite a T record at offset, which must be a multiple of 4. Only legal if the record - * was writtern atomically using the write methods below. + * was written atomically using the write methods below. */ template<typename T> void overwriteTAt(size_t offset, const T& value) { SkASSERT(SkAlign4(offset) == offset); SkASSERT(offset < fUsed); + SkASSERT(fSnapshot.get() == NULL); *(T*)(fData + offset) = value; } @@ -230,6 +233,18 @@ public: return stream->read(this->reservePad(length), length); } + /** + * Captures a snapshot of the data as it is right now, and return it. + * Multiple calls without intervening writes may return the same SkData, + * but this is not guaranteed. + * Future appends will not affect the returned buffer. + * It is illegal to call overwriteTAt after this without an intervening + * append. It may cause the snapshot buffer to be corrupted. + * Callers must unref the returned SkData. + * This is not thread safe, it should only be called on the writing thread, + * the result however can be shared across threads. + */ + SkData* snapshotAsData() const; private: void growToAtLeast(size_t size); @@ -238,6 +253,7 @@ private: size_t fUsed; // Number of bytes written. void* fExternal; // Unmanaged memory block. SkAutoTMalloc<uint8_t> fInternal; // Managed memory block. + SkAutoTUnref<SkData> fSnapshot; // Holds the result of last asData. }; /** diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp index 05c2c4e56b..d61e616c4d 100644 --- a/src/core/SkPicturePlayback.cpp +++ b/src/core/SkPicturePlayback.cpp @@ -70,10 +70,12 @@ SkPicturePlayback::SkPicturePlayback(const SkPictureRecord& record, bool deepCop record.validate(record.writeStream().bytesWritten(), 0); const SkWriter32& writer = record.writeStream(); init(); + SkASSERT(!fOpData); if (writer.bytesWritten() == 0) { fOpData = SkData::NewEmpty(); return; } + fOpData = writer.snapshotAsData(); fBoundingHierarchy = record.fBoundingHierarchy; fStateTree = record.fStateTree; @@ -85,14 +87,6 @@ SkPicturePlayback::SkPicturePlayback(const SkPictureRecord& record, bool deepCop fBoundingHierarchy->flushDeferredInserts(); } - { - size_t size = writer.bytesWritten(); - void* buffer = sk_malloc_throw(size); - writer.flatten(buffer); - SkASSERT(!fOpData); - fOpData = SkData::NewFromMalloc(buffer, size); - } - // copy over the refcnt dictionary to our reader record.fFlattenableHeap.setupPlaybacks(); diff --git a/src/core/SkWriter32.cpp b/src/core/SkWriter32.cpp index fe33e4a39c..c7bfd92d56 100644 --- a/src/core/SkWriter32.cpp +++ b/src/core/SkWriter32.cpp @@ -74,4 +74,32 @@ void SkWriter32::growToAtLeast(size_t size) { // we were external, so copy in the data memcpy(fData, fExternal, fUsed); } + // Invalidate the snapshot, we know it is no longer useful. + fSnapshot.reset(NULL); +} + +SkData* SkWriter32::snapshotAsData() const { + // get a non const version of this, we are only conceptually const + SkWriter32& mutable_this = *const_cast<SkWriter32*>(this); + // we use size change detection to invalidate the cached data + if ((fSnapshot.get() != NULL) && (fSnapshot->size() != fUsed)) { + mutable_this.fSnapshot.reset(NULL); + } + if (fSnapshot.get() == NULL) { + uint8_t* buffer = NULL; + if ((fExternal != NULL) && (fData == fExternal)) { + // We need to copy to an allocated buffer before returning. + buffer = (uint8_t*)sk_malloc_throw(fUsed); + memcpy(buffer, fData, fUsed); + } else { + buffer = mutable_this.fInternal.detach(); + // prepare us to do copy on write, by pretending the data buffer + // is external and size limited + mutable_this.fData = buffer; + mutable_this.fCapacity = fUsed; + mutable_this.fExternal = buffer; + } + mutable_this.fSnapshot.reset(SkData::NewFromMalloc(buffer, fUsed)); + } + return SkRef(fSnapshot.get()); // Take an extra ref for the caller. } diff --git a/tests/Writer32Test.cpp b/tests/Writer32Test.cpp index a471099b38..f9b6f6a03f 100644 --- a/tests/Writer32Test.cpp +++ b/tests/Writer32Test.cpp @@ -280,3 +280,36 @@ DEF_TEST(Writer32_misc, reporter) { test_ptr(reporter); test_rewind(reporter); } + +DEF_TEST(Writer32_snapshot, reporter) { + int32_t array[] = { 1, 2, 4, 11 }; + SkSWriter32<sizeof(array) + 4> writer; + writer.write(array, sizeof(array)); + check_contents(reporter, writer, array, sizeof(array)); + const void* beforeData = writer.contiguousArray(); + SkAutoDataUnref snapshot(writer.snapshotAsData()); + // check the snapshot forced a copy of the static data + REPORTER_ASSERT(reporter, snapshot->data() != beforeData); + REPORTER_ASSERT(reporter, snapshot->size() == writer.bytesWritten()); +} + +DEF_TEST(Writer32_snapshot_dynamic, reporter) { + int32_t array[] = { 1, 2, 4, 11 }; + SkWriter32 writer; + writer.write(array, sizeof(array)); + check_contents(reporter, writer, array, sizeof(array)); + // force a capacity increase so we can test COW behaviour + writer.write(array, sizeof(array)); + writer.rewindToOffset(sizeof(array)); + const void* beforeData = writer.contiguousArray(); + SkAutoDataUnref snapshot(writer.snapshotAsData()); + // check the snapshot still points to the same data as the writer + REPORTER_ASSERT(reporter, writer.contiguousArray() == beforeData); + REPORTER_ASSERT(reporter, snapshot->data() == beforeData); + REPORTER_ASSERT(reporter, snapshot->size() == writer.bytesWritten()); + // write more data that would fit in the buffer + writer.write(array, sizeof(array)); + // test it triggered COW anyway + REPORTER_ASSERT(reporter, writer.contiguousArray() != beforeData); +} + |