Templetized SkWriter32 readTAt() & overwriteTAt()
Convert SkWriter32::{read,write}32At() to ::{read,overwrite}TAt<>() to allow
peeking/updating arbitrary records.
BUG=skia:
R=mtklein@google.com, reed@google.com, robertphillips@google.com, iancottrell@chromium.org
Author: fmalita@chromium.org
Review URL: https://codereview.chromium.org/130913018
git-svn-id: http://skia.googlecode.com/svn/trunk@13416 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/include/core/SkWriter32.h b/include/core/SkWriter32.h
index 7d8d764..586fe9f 100644
--- a/include/core/SkWriter32.h
+++ b/include/core/SkWriter32.h
@@ -73,17 +73,26 @@
return (uint32_t*)(fData + offset);
}
- // Read or write 4 bytes at offset, which must be a multiple of 4 <= size().
- uint32_t read32At(size_t offset) {
+ /**
+ * 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.
+ */
+ template<typename T>
+ const T& readTAt(size_t offset) const {
SkASSERT(SkAlign4(offset) == offset);
SkASSERT(offset < fUsed);
- return *(uint32_t*)(fData + offset);
+ return *(T*)(fData + offset);
}
- void write32At(size_t offset, uint32_t val) {
+ /**
+ * 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.
+ */
+ template<typename T>
+ void overwriteTAt(size_t offset, const T& value) {
SkASSERT(SkAlign4(offset) == offset);
SkASSERT(offset < fUsed);
- *(uint32_t*)(fData + offset) = val;
+ *(T*)(fData + offset) = value;
}
bool writeBool(bool value) {
@@ -168,14 +177,11 @@
* filled in with zeroes.
*/
uint32_t* reservePad(size_t size) {
- uint32_t* p = this->reserve(SkAlign4(size));
- uint8_t* tail = (uint8_t*)p + size;
- switch (SkAlign4(size) - size) {
- default: SkDEBUGFAIL("SkAlign4(x) - x should always be 0, 1, 2, or 3.");
- case 3: *tail++ = 0x00; // fallthrough is intentional
- case 2: *tail++ = 0x00; // fallthrough is intentional
- case 1: *tail++ = 0x00;
- case 0: ;/*nothing to do*/
+ size_t alignedSize = SkAlign4(size);
+ uint32_t* p = this->reserve(alignedSize);
+ if (alignedSize != size) {
+ SkASSERT(alignedSize >= 4);
+ p[alignedSize / 4 - 1] = 0;
}
return p;
}
diff --git a/src/core/SkMatrixClipStateMgr.cpp b/src/core/SkMatrixClipStateMgr.cpp
index b04d7ff..3b4f787 100644
--- a/src/core/SkMatrixClipStateMgr.cpp
+++ b/src/core/SkMatrixClipStateMgr.cpp
@@ -106,7 +106,7 @@
}
// SkDEBUGCODE(uint32_t peek = writer->read32At(curClip.fOffset);)
// SkASSERT(-1 == peek);
- writer->write32At(curClip.fOffset, restoreOffset);
+ writer->overwriteTAt(curClip.fOffset, restoreOffset);
SkDEBUGCODE(curClip.fOffset = -1;)
}
}
diff --git a/src/core/SkPictureRecord.cpp b/src/core/SkPictureRecord.cpp
index 0b6f9a6..042e6a3 100644
--- a/src/core/SkPictureRecord.cpp
+++ b/src/core/SkPictureRecord.cpp
@@ -226,13 +226,13 @@
* Read the op code from 'offset' in 'writer' and extract the size too.
*/
static DrawType peek_op_and_size(SkWriter32* writer, int32_t offset, uint32_t* size) {
- uint32_t peek = writer->read32At(offset);
+ uint32_t peek = writer->readTAt<uint32_t>(offset);
uint32_t op;
UNPACK_8_24(peek, op, *size);
if (MASK_24 == *size) {
// size required its own slot right after the op code
- *size = writer->read32At(offset+kUInt32Size);
+ *size = writer->readTAt<uint32_t>(offset + kUInt32Size);
}
return (DrawType) op;
}
@@ -335,7 +335,7 @@
// back up to the save block
// TODO: add a stack to track save*/restore offsets rather than searching backwards
while (offset > 0) {
- offset = writer->read32At(offset);
+ offset = writer->readTAt<uint32_t>(offset);
}
int pattern[] = { SAVE_LAYER, kDRAW_BITMAP_FLAVOR, /* RESTORE */ };
@@ -359,8 +359,8 @@
* field alone so the NOOP can be skipped later.
*/
static void convert_command_to_noop(SkWriter32* writer, uint32_t offset) {
- uint32_t command = writer->read32At(offset);
- writer->write32At(offset, (command & MASK_24) | (NOOP << 24));
+ uint32_t command = writer->readTAt<uint32_t>(offset);
+ writer->overwriteTAt(offset, (command & MASK_24) | (NOOP << 24));
}
/*
@@ -381,8 +381,8 @@
uint32_t slPaintOffset = getPaintOffset(SAVE_LAYER, saveLayerInfo.fSize);
// we have a match, now we need to get the paints involved
- uint32_t dbmPaintId = writer->read32At(dbmInfo.fOffset+dbmPaintOffset);
- uint32_t saveLayerPaintId = writer->read32At(saveLayerInfo.fOffset+slPaintOffset);
+ uint32_t dbmPaintId = writer->readTAt<uint32_t>(dbmInfo.fOffset + dbmPaintOffset);
+ uint32_t saveLayerPaintId = writer->readTAt<uint32_t>(saveLayerInfo.fOffset + slPaintOffset);
if (0 == saveLayerPaintId) {
// In this case the saveLayer/restore isn't needed at all - just kill the saveLayer
@@ -395,7 +395,7 @@
// In this case just make the DBM* use the saveLayer's paint, kill the saveLayer
// and signal the caller (by returning true) to not add the RESTORE op
convert_command_to_noop(writer, saveLayerInfo.fOffset);
- writer->write32At(dbmInfo.fOffset+dbmPaintOffset, saveLayerPaintId);
+ writer->overwriteTAt(dbmInfo.fOffset + dbmPaintOffset, saveLayerPaintId);
return true;
}
@@ -428,7 +428,7 @@
// kill the saveLayer and alter the DBMR2R's paint to be the modified one
convert_command_to_noop(writer, saveLayerInfo.fOffset);
- writer->write32At(dbmInfo.fOffset+dbmPaintOffset, data->index());
+ writer->overwriteTAt(dbmInfo.fOffset + dbmPaintOffset, data->index());
return true;
}
@@ -449,7 +449,7 @@
// back up to the save block
// TODO: add a stack to track save*/restore offsets rather than searching backwards
while (offset > 0) {
- offset = writer->read32At(offset);
+ offset = writer->readTAt<uint32_t>(offset);
}
int pattern[] = { SAVE_LAYER, SAVE, CLIP_RECT, kDRAW_BITMAP_FLAVOR, RESTORE, /* RESTORE */ };
@@ -486,7 +486,7 @@
// back up to the save block
while (offset > 0) {
- offset = writer->read32At(offset);
+ offset = writer->readTAt<uint32_t>(offset);
}
// now offset points to a save
@@ -501,7 +501,7 @@
SkASSERT(kSaveSize == opSize);
// get the save flag (last 4-bytes of the space allocated for the opSize)
- SkCanvas::SaveFlags saveFlags = (SkCanvas::SaveFlags) writer->read32At(offset+4);
+ SkCanvas::SaveFlags saveFlags = (SkCanvas::SaveFlags) writer->readTAt<uint32_t>(offset + 4);
if (SkCanvas::kMatrixClip_SaveFlag != saveFlags) {
// This function's optimization is only correct for kMatrixClip style saves.
// TODO: set checkMatrix & checkClip booleans here and then check for the
@@ -769,8 +769,8 @@
void SkPictureRecord::fillRestoreOffsetPlaceholdersForCurrentStackLevel(uint32_t restoreOffset) {
int32_t offset = fRestoreOffsetStack.top();
while (offset > 0) {
- uint32_t peek = fWriter.read32At(offset);
- fWriter.write32At(offset, restoreOffset);
+ uint32_t peek = fWriter.readTAt<uint32_t>(offset);
+ fWriter.overwriteTAt(offset, restoreOffset);
offset = peek;
}
diff --git a/src/core/SkWriteBuffer.cpp b/src/core/SkWriteBuffer.cpp
index a7805b5..450c30e 100644
--- a/src/core/SkWriteBuffer.cpp
+++ b/src/core/SkWriteBuffer.cpp
@@ -321,5 +321,5 @@
flattenable->flatten(*this);
size_t objSize = fWriter.bytesWritten() - offset;
// record the obj's size
- fWriter.write32At(offset - sizeof(uint32_t), SkToU32(objSize));
+ fWriter.overwriteTAt(offset - sizeof(uint32_t), SkToU32(objSize));
}
diff --git a/src/core/SkWriter32.cpp b/src/core/SkWriter32.cpp
index 041e2fe..46150ee 100644
--- a/src/core/SkWriter32.cpp
+++ b/src/core/SkWriter32.cpp
@@ -44,17 +44,13 @@
if ((long)len < 0) {
len = strlen(str);
}
- this->write32(len);
- // add 1 since we also write a terminating 0
- size_t alignedLen = SkAlign4(len + 1);
- char* ptr = (char*)this->reserve(alignedLen);
- {
- // Write the terminating 0 and fill in the rest with zeroes
- uint32_t* padding = (uint32_t*)(ptr + (alignedLen - 4));
- *padding = 0;
- }
- // Copy the string itself.
- memcpy(ptr, str, len);
+
+ // [ 4 byte len ] [ str ... ] [1 - 4 \0s]
+ uint32_t* ptr = this->reservePad(sizeof(uint32_t) + len + 1);
+ *ptr = len;
+ char* chars = (char*)(ptr + 1);
+ memcpy(chars, str, len);
+ chars[len] = '\0';
}
size_t SkWriter32::WriteStringSize(const char* str, size_t len) {
diff --git a/tests/Writer32Test.cpp b/tests/Writer32Test.cpp
index 4d421c8..a471099 100644
--- a/tests/Writer32Test.cpp
+++ b/tests/Writer32Test.cpp
@@ -102,7 +102,7 @@
for (size_t i = 0; i < SK_ARRAY_COUNT(data); ++i) {
REPORTER_ASSERT(reporter, i*4 == writer->bytesWritten());
writer->write32(data[i]);
- REPORTER_ASSERT(reporter, data[i] == writer->read32At(i*4));
+ REPORTER_ASSERT(reporter, data[i] == writer->readTAt<uint32_t>(i * 4));
}
char buffer[sizeof(data)];
@@ -184,6 +184,43 @@
}
}
+static void testOverwriteT(skiatest::Reporter* reporter, SkWriter32* writer) {
+ const size_t padding = 64;
+
+ const uint32_t uint1 = 0x12345678;
+ const uint32_t uint2 = 0x98765432;
+ const SkScalar scalar1 = 1234.5678f;
+ const SkScalar scalar2 = 9876.5432f;
+ const SkRect rect1 = SkRect::MakeXYWH(1, 2, 3, 4);
+ const SkRect rect2 = SkRect::MakeXYWH(5, 6, 7, 8);
+
+ for (size_t i = 0; i < (padding / 4); ++i) {
+ writer->write32(0);
+ }
+
+ writer->write32(uint1);
+ writer->writeRect(rect1);
+ writer->writeScalar(scalar1);
+
+ for (size_t i = 0; i < (padding / 4); ++i) {
+ writer->write32(0);
+ }
+
+ REPORTER_ASSERT(reporter, writer->readTAt<uint32_t>(padding) == uint1);
+ REPORTER_ASSERT(reporter, writer->readTAt<SkRect>(padding + sizeof(uint32_t)) == rect1);
+ REPORTER_ASSERT(reporter, writer->readTAt<SkScalar>(
+ padding + sizeof(uint32_t) + sizeof(SkRect)) == scalar1);
+
+ writer->overwriteTAt(padding, uint2);
+ writer->overwriteTAt(padding + sizeof(uint32_t), rect2);
+ writer->overwriteTAt(padding + sizeof(uint32_t) + sizeof(SkRect), scalar2);
+
+ REPORTER_ASSERT(reporter, writer->readTAt<uint32_t>(padding) == uint2);
+ REPORTER_ASSERT(reporter, writer->readTAt<SkRect>(padding + sizeof(uint32_t)) == rect2);
+ REPORTER_ASSERT(reporter, writer->readTAt<SkScalar>(
+ padding + sizeof(uint32_t) + sizeof(SkRect)) == scalar2);
+}
+
DEF_TEST(Writer32_dynamic, reporter) {
SkWriter32 writer;
test1(reporter, &writer);
@@ -193,6 +230,9 @@
writer.reset();
testWritePad(reporter, &writer);
+
+ writer.reset();
+ testOverwriteT(reporter, &writer);
}
DEF_TEST(Writer32_contiguous, reporter) {
@@ -216,6 +256,9 @@
writer.reset();
testWritePad(reporter, &writer);
+
+ writer.reset();
+ testOverwriteT(reporter, &writer);
}
DEF_TEST(Writer32_large, reporter) {
@@ -226,6 +269,9 @@
writer.reset();
testWritePad(reporter, &writer);
+
+ writer.reset();
+ testOverwriteT(reporter, &writer);
}
DEF_TEST(Writer32_misc, reporter) {