aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81>2014-03-12 17:04:28 +0000
committerGravatar commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81>2014-03-12 17:04:28 +0000
commitc30dcb9b128887c7e16afe32fdf35105cc42380b (patch)
tree97f55d320d7191e0f876ff9dc3ad2d1528064e62
parent84cd099704b3896ca66081a96508572a924f850c (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.h20
-rw-r--r--src/core/SkPicturePlayback.cpp10
-rw-r--r--src/core/SkWriter32.cpp28
-rw-r--r--tests/Writer32Test.cpp33
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);
+}
+