Remove bogus ability for creating an SkPicturePlayback to fail.
Change SkPicturePlayback::parseBufferTag to return void, since
it can never return false.
Change SkPicturePlayback::parseStreamTag to return void, since
the only way it can return false is if parseBufferTag returns
false, or if creating a sub picture failed, both of which are
nonsensical.
Due to the above, there is no reason for creating an
SkPicturePlayback to fail, so remove the isValid parameter.
Update subclasses in SkDebuggerGUI.
Review URL: https://codereview.appspot.com/7388050
git-svn-id: http://skia.googlecode.com/svn/trunk@7844 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/debugger/QT/SkDebuggerGUI.cpp b/debugger/QT/SkDebuggerGUI.cpp
index e3050cb..f1f4694 100644
--- a/debugger/QT/SkDebuggerGUI.cpp
+++ b/debugger/QT/SkDebuggerGUI.cpp
@@ -143,10 +143,10 @@
// offsets to individual commands.
class SkTimedPicturePlayback : public SkPicturePlayback {
public:
- SkTimedPicturePlayback(SkStream* stream, const SkPictInfo& info, bool* isValid,
+ SkTimedPicturePlayback(SkStream* stream, const SkPictInfo& info,
SkPicture::InstallPixelRefProc proc, const SkTDArray<size_t>& offsets,
const SkTDArray<bool>& deletedCommands)
- : INHERITED(stream, info, isValid, proc)
+ : INHERITED(stream, info, proc)
, fOffsets(offsets)
, fSkipCommands(deletedCommands)
, fTot(0.0)
@@ -270,14 +270,8 @@
}
if (stream->readBool()) {
- bool isValid = false;
fPlayback = SkNEW_ARGS(SkTimedPicturePlayback,
- (stream, info, &isValid, proc, offsets, deletedCommands));
- if (!isValid) {
- SkDELETE(fPlayback);
- fPlayback = NULL;
- return;
- }
+ (stream, info, proc, offsets, deletedCommands));
}
// do this at the end, so that they will be zero if we hit an error.
@@ -924,9 +918,9 @@
// These are needed by the profiling system.
class SkOffsetPicturePlayback : public SkPicturePlayback {
public:
- SkOffsetPicturePlayback(SkStream* stream, const SkPictInfo& info, bool* isValid,
+ SkOffsetPicturePlayback(SkStream* stream, const SkPictInfo& info,
SkPicture::InstallPixelRefProc proc)
- : INHERITED(stream, info, isValid, proc) {
+ : INHERITED(stream, info, proc) {
}
const SkTDArray<size_t>& offsets() const { return fOffsets; }
@@ -964,13 +958,7 @@
}
if (stream->readBool()) {
- bool isValid = false;
- fPlayback = SkNEW_ARGS(SkOffsetPicturePlayback, (stream, info, &isValid, proc));
- if (!isValid) {
- SkDELETE(fPlayback);
- fPlayback = NULL;
- return;
- }
+ fPlayback = SkNEW_ARGS(SkOffsetPicturePlayback, (stream, info, proc));
}
// do this at the end, so that they will be zero if we hit an error.
diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index 381bbae..1ff5865 100644
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -290,13 +290,7 @@
}
if (stream->readBool()) {
- bool isValid = false;
- fPlayback = SkNEW_ARGS(SkPicturePlayback, (stream, info, &isValid, proc));
- if (!isValid) {
- SkDELETE(fPlayback);
- fPlayback = NULL;
- return;
- }
+ fPlayback = SkNEW_ARGS(SkPicturePlayback, (stream, info, proc));
}
// do this at the end, so that they will be zero if we hit an error.
diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index 9ab7b01..93e5cf1 100644
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -473,7 +473,7 @@
return rbMask;
}
-bool SkPicturePlayback::parseStreamTag(SkStream* stream, const SkPictInfo& info, uint32_t tag,
+void SkPicturePlayback::parseStreamTag(SkStream* stream, const SkPictInfo& info, uint32_t tag,
size_t size, SkPicture::InstallPixelRefProc proc) {
/*
* By the time we encounter BUFFER_SIZE_TAG, we need to have already seen
@@ -514,23 +514,15 @@
case PICT_PICTURE_TAG: {
fPictureCount = size;
fPictureRefs = SkNEW_ARRAY(SkPicture*, fPictureCount);
- bool success = true;
- int i = 0;
- for ( ; i < fPictureCount; i++) {
+ bool success;
+ for (int i = 0; i < fPictureCount; i++) {
fPictureRefs[i] = SkNEW_ARGS(SkPicture, (stream, &success, proc));
- if (!success) {
- break;
- }
- }
- if (!success) {
- // Delete all of the pictures that were already created (up through i):
- for (int j = 0; j <= i; j++) {
- fPictureRefs[j]->unref();
- }
- // Delete the array
- SkDELETE_ARRAY(fPictureRefs);
- fPictureCount = 0;
- return false;
+ // Success can only be false if PICTURE_VERSION does not match
+ // (which should never happen from here, since a sub picture will
+ // have the same PICTURE_VERSION as its parent) or if stream->read
+ // returns 0. In the latter case, we have a bug when writing the
+ // picture to begin with, which will be alerted to here.
+ SkASSERT(success);
}
} break;
case PICT_BUFFER_SIZE_TAG: {
@@ -547,17 +539,14 @@
while (!buffer.eof()) {
tag = buffer.readUInt();
size = buffer.readUInt();
- if (!this->parseBufferTag(buffer, tag, size)) {
- return false;
- }
+ this->parseBufferTag(buffer, tag, size);
}
SkDEBUGCODE(haveBuffer = true;)
} break;
}
- return true; // success
}
-bool SkPicturePlayback::parseBufferTag(SkOrderedReadBuffer& buffer,
+void SkPicturePlayback::parseBufferTag(SkOrderedReadBuffer& buffer,
uint32_t tag, size_t size) {
switch (tag) {
case PICT_BITMAP_BUFFER_TAG: {
@@ -592,14 +581,12 @@
}
} break;
}
- return true; // success
}
-SkPicturePlayback::SkPicturePlayback(SkStream* stream, const SkPictInfo& info, bool* isValid,
+SkPicturePlayback::SkPicturePlayback(SkStream* stream, const SkPictInfo& info,
SkPicture::InstallPixelRefProc proc) {
this->init();
- *isValid = false; // wait until we're done parsing to mark as true
for (;;) {
uint32_t tag = stream->readU32();
if (PICT_EOF_TAG == tag) {
@@ -607,11 +594,8 @@
}
uint32_t size = stream->readU32();
- if (!this->parseStreamTag(stream, info, tag, size, proc)) {
- return; // we're invalid
- }
+ this->parseStreamTag(stream, info, tag, size, proc);
}
- *isValid = true;
}
///////////////////////////////////////////////////////////////////////////////
diff --git a/src/core/SkPicturePlayback.h b/src/core/SkPicturePlayback.h
index 92c099f..6acc0ae 100644
--- a/src/core/SkPicturePlayback.h
+++ b/src/core/SkPicturePlayback.h
@@ -62,7 +62,7 @@
SkPicturePlayback();
SkPicturePlayback(const SkPicturePlayback& src, SkPictCopyInfo* deepCopyInfo = NULL);
explicit SkPicturePlayback(const SkPictureRecord& record, bool deepCopy = false);
- SkPicturePlayback(SkStream*, const SkPictInfo&, bool* isValid, SkPicture::InstallPixelRefProc);
+ SkPicturePlayback(SkStream*, const SkPictInfo&, SkPicture::InstallPixelRefProc);
virtual ~SkPicturePlayback();
@@ -189,9 +189,9 @@
#endif
private: // these help us with reading/writing
- bool parseStreamTag(SkStream*, const SkPictInfo&, uint32_t tag, size_t size,
+ void parseStreamTag(SkStream*, const SkPictInfo&, uint32_t tag, size_t size,
SkPicture::InstallPixelRefProc);
- bool parseBufferTag(SkOrderedReadBuffer&, uint32_t tag, size_t size);
+ void parseBufferTag(SkOrderedReadBuffer&, uint32_t tag, size_t size);
void flattenToBuffer(SkOrderedWriteBuffer&) const;
private: