More SkPicture cleanup

src/core/SkPicture.cpp

Index: src/core/SkPicture.cpp
diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index 5231cb9570b3af97c4694b1c859894620c0ba1d2..e2514c67d6e858fa8fd4d6f4227a1df4d9f6031b 100644
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -257,41 +257,14 @@ bool SkPicture::Analysis::suitableForGpuRasterization(const char** reason,
 
 ///////////////////////////////////////////////////////////////////////////////
 
-// fRecord OK
-SkPicture::SkPicture(SkScalar width, SkScalar height,
-                     const SkPictureRecord& record,
-                     bool deepCopyOps)
-    : fCullWidth(width)
-    , fCullHeight(height)
-    , fAnalysis() {
-    this->needsNewGenID();
-
-    SkPictInfo info;
-    this->createHeader(&info);
-    fData.reset(SkNEW_ARGS(SkPictureData, (record, info, deepCopyOps)));
-}
-
-// Create an SkPictureData-backed SkPicture from an SkRecord.
-// This for compatibility with serialization code only.  This is not cheap.
-SkPicture* SkPicture::Backport(const SkRecord& src, const SkRect& cullRect) {
-    SkPictureRecord rec(SkISize::Make(cullRect.width(), cullRect.height()), 0/*flags*/);
-    rec.beginRecording();
-        SkRecordDraw(src, &rec, NULL/*bbh*/, NULL/*callback*/);
-    rec.endRecording();
-    return SkNEW_ARGS(SkPicture, (cullRect.width(), cullRect.height(), rec, false/*deepCopyOps*/));
-}
-
-// fRecord OK
 SkPicture::~SkPicture() {
     this->callDeletionListeners();
 }
 
-// fRecord OK
 void SkPicture::EXPERIMENTAL_addAccelData(const SkPicture::AccelData* data) const {
     fAccelData.reset(SkRef(data));
 }
 
-// fRecord OK
 const SkPicture::AccelData* SkPicture::EXPERIMENTAL_getAccelData(
         SkPicture::AccelData::Key key) const {
     if (fAccelData.get() && fAccelData->getKey() == key) {
@@ -300,7 +273,6 @@ const SkPicture::AccelData* SkPicture::EXPERIMENTAL_getAccelData(
     return NULL;
 }
 
-// fRecord OK
 SkPicture::AccelData::Domain SkPicture::AccelData::GenerateDomain() {
     static int32_t gNextID = 0;
 
@@ -314,23 +286,15 @@ SkPicture::AccelData::Domain SkPicture::AccelData::GenerateDomain() {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-// fRecord OK
 void SkPicture::playback(SkCanvas* canvas, SkDrawPictureCallback* callback) const {
     SkASSERT(canvas);
-    SkASSERT(fData.get() || fRecord.get());
 
-    if (fData.get()) {
-        SkPicturePlayback playback(this);
-        playback.draw(canvas, callback);
-    }
-    if (fRecord.get()) {
-        // If the query contains the whole picture, don't bother with the BBH.
-        SkRect clipBounds = { 0, 0, 0, 0 };
-        (void)canvas->getClipBounds(&clipBounds);
-        const bool useBBH = !clipBounds.contains(this->cullRect());
+    // If the query contains the whole picture, don't bother with the BBH.
+    SkRect clipBounds = { 0, 0, 0, 0 };
+    (void)canvas->getClipBounds(&clipBounds);
+    const bool useBBH = !clipBounds.contains(this->cullRect());
 
-        SkRecordDraw(*fRecord, canvas, useBBH ? fBBH.get() : NULL, callback);
-    }
+    SkRecordDraw(*fRecord, canvas, useBBH ? fBBH.get() : NULL, callback);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -339,7 +303,6 @@ void SkPicture::playback(SkCanvas* canvas, SkDrawPictureCallback* callback) cons
 
 static const char kMagic[] = { 's', 'k', 'i', 'a', 'p', 'i', 'c', 't' };
 
-// fRecord OK
 bool SkPicture::IsValidPictInfo(const SkPictInfo& info) {
     if (0 != memcmp(info.fMagic, kMagic, sizeof(kMagic))) {
         return false;
@@ -353,7 +316,6 @@ bool SkPicture::IsValidPictInfo(const SkPictInfo& info) {
     return true;
 }
 
-// fRecord OK
 bool SkPicture::InternalOnly_StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) {
     if (NULL == stream) {
         return false;
@@ -397,7 +359,6 @@ bool SkPicture::InternalOnly_StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) {
     return true;
 }
 
-// fRecord OK
 bool SkPicture::InternalOnly_BufferIsSKP(SkReadBuffer* buffer, SkPictInfo* pInfo) {
     // Check magic bytes.
     SkPictInfo info;
@@ -434,66 +395,36 @@ bool SkPicture::InternalOnly_BufferIsSKP(SkReadBuffer* buffer, SkPictInfo* pInfo
     return true;
 }
 
-// fRecord OK
-SkPicture::SkPicture(SkPictureData* data, SkScalar width, SkScalar height)
-    : fData(data)
-    , fCullWidth(width)
-    , fCullHeight(height)
-    , fAnalysis() {
-    this->needsNewGenID();
-}
-
-SkPicture* SkPicture::Forwardport(const SkPicture& src) {
-    SkAutoTDelete<SkRecord> record(SkNEW(SkRecord));
-    SkRecorder canvas(record.get(), src.cullRect().width(), src.cullRect().height());
-    src.playback(&canvas);
-    return SkNEW_ARGS(SkPicture, (src.cullRect().width(), src.cullRect().height(),
-                                  record.detach(), NULL/*bbh*/));
+SkPicture* SkPicture::Forwardport(const SkPictInfo& info, const SkPictureData* data) {
+    if (!data) {
+        return NULL;
+    }
+    SkPicturePlayback playback(data);
+    SkPictureRecorder r;
+    playback.draw(r.beginRecording(SkScalarCeilToInt(info.fCullRect.width()),
+                                   SkScalarCeilToInt(info.fCullRect.height())),
+                  NULL/*no callback*/);
+    return r.endRecording();
 }
 
-// fRecord OK
 SkPicture* SkPicture::CreateFromStream(SkStream* stream, InstallPixelRefProc proc) {
     SkPictInfo info;
-
-    if (!InternalOnly_StreamIsSKP(stream, &info)) {
+    if (!InternalOnly_StreamIsSKP(stream, &info) || !stream->readBool()) {
         return NULL;
     }
-
-    // Check to see if there is a playback to recreate.
-    if (stream->readBool()) {
-        SkPictureData* data = SkPictureData::CreateFromStream(stream, info, proc);
-        if (NULL == data) {
-            return NULL;
-        }
-        const SkPicture src(data, info.fCullRect.width(), info.fCullRect.height());
-        return Forwardport(src);
-    }
-
-    return NULL;
+    SkAutoTDelete<SkPictureData> data(SkPictureData::CreateFromStream(stream, info, proc));
+    return Forwardport(info, data);
 }
 
-// fRecord OK
 SkPicture* SkPicture::CreateFromBuffer(SkReadBuffer& buffer) {
     SkPictInfo info;
-
-    if (!InternalOnly_BufferIsSKP(&buffer, &info)) {
+    if (!InternalOnly_BufferIsSKP(&buffer, &info) || !buffer.readBool()) {
         return NULL;
     }
-
-    // Check to see if there is a playback to recreate.
-    if (buffer.readBool()) {
-        SkPictureData* data = SkPictureData::CreateFromBuffer(buffer, info);
-        if (NULL == data) {
-            return NULL;
-        }
-        const SkPicture src(data, info.fCullRect.width(), info.fCullRect.height());
-        return Forwardport(src);
-    }
-
-    return NULL;
+    SkAutoTDelete<SkPictureData> data(SkPictureData::CreateFromBuffer(buffer, info));
+    return Forwardport(info, data);
 }
 
-// fRecord OK
 void SkPicture::createHeader(SkPictInfo* info) const {
     // Copy magic bytes at the beginning of the header
     SkASSERT(sizeof(kMagic) == 8);
@@ -512,23 +443,22 @@ void SkPicture::createHeader(SkPictInfo* info) const {
     }
 }
 
-// fRecord OK
-void SkPicture::serialize(SkWStream* stream, EncodeBitmap encoder) const {
-    const SkPictureData* data = fData.get();
+// This for compatibility with serialization code only.  This is not cheap.
+SkPictureData* SkPicture::Backport(const SkRecord& src, const SkPictInfo& info) {
+    SkPictureRecord rec(SkISize::Make(info.fCullRect.width(), info.fCullRect.height()), 0/*flags*/);
+    rec.beginRecording();
+        SkRecordDraw(src, &rec, NULL/*bbh*/, NULL/*callback*/);
+    rec.endRecording();
+    return SkNEW_ARGS(SkPictureData, (rec, info, false/*deep copy ops?*/));
+}
 
-    // If we're a new-format picture, backport to old format for serialization.
-    SkAutoTDelete<SkPicture> oldFormat;
-    if (NULL == data && fRecord.get()) {
-        oldFormat.reset(Backport(*fRecord, this->cullRect()));
-        data = oldFormat->fData.get();
-        SkASSERT(data);
-    }
 
+void SkPicture::serialize(SkWStream* stream, EncodeBitmap encoder) const {
     SkPictInfo info;
     this->createHeader(&info);
-    SkASSERT(sizeof(SkPictInfo) == 32);
-    stream->write(&info, sizeof(info));
+    SkAutoTDelete<SkPictureData> data(Backport(*fRecord, info));
 
+    stream->write(&info, sizeof(info));
     if (data) {
         stream->writeBool(true);
         data->serialize(stream, encoder);
@@ -537,25 +467,15 @@ void SkPicture::serialize(SkWStream* stream, EncodeBitmap encoder) const {
     }
 }
 
-// fRecord OK
 void SkPicture::flatten(SkWriteBuffer& buffer) const {
-    const SkPictureData* data = fData.get();
-
-    // If we're a new-format picture, backport to old format for serialization.
-    SkAutoTDelete<SkPicture> oldFormat;
-    if (NULL == data && fRecord.get()) {
-        oldFormat.reset(Backport(*fRecord, this->cullRect()));
-        data = oldFormat->fData.get();
-        SkASSERT(data);
-    }
-
     SkPictInfo info;
     this->createHeader(&info);
+    SkAutoTDelete<SkPictureData> data(Backport(*fRecord, info));
+
     buffer.writeByteArray(&info.fMagic, sizeof(info.fMagic));
     buffer.writeUInt(info.fVersion);
     buffer.writeRect(info.fCullRect);
     buffer.writeUInt(info.fFlags);
-
     if (data) {
         buffer.writeBool(true);
         data->flatten(buffer);
@@ -565,51 +485,19 @@ void SkPicture::flatten(SkWriteBuffer& buffer) const {
 }
 
 #if SK_SUPPORT_GPU
-// fRecord OK
-bool SkPicture::suitableForGpuRasterization(GrContext* context, const char **reason) const {
-    if (fRecord.get()) {
-        return fAnalysis.suitableForGpuRasterization(reason, 0);
-    }
-    if (NULL == fData.get()) {
-        if (reason) {
-            *reason = "Missing internal data.";
-        }
-        return false;
-    }
-
-    return fData->suitableForGpuRasterization(context, reason);
+bool SkPicture::suitableForGpuRasterization(GrContext*, const char **reason) const {
+    return fAnalysis.suitableForGpuRasterization(reason, 0);
 }
 #endif
 
-// fRecord OK
-bool SkPicture::hasText() const {
-    if (fRecord.get()) {
-        return fAnalysis.fHasText;
-    }
-    if (fData.get()) {
-        return fData->hasText();
-    }
-    SkFAIL("Unreachable");
-    return false;
-}
+bool SkPicture::hasText()             const { return fAnalysis.fHasText; }
+bool SkPicture::willPlayBackBitmaps() const { return fAnalysis.fWillPlaybackBitmaps; }
+int  SkPicture::approximateOpCount()  const { return fRecord->count(); }
 
-// fRecord OK
-bool SkPicture::willPlayBackBitmaps() const {
-    if (fRecord.get()) {
-        return fAnalysis.fWillPlaybackBitmaps;
-    }
-    if (fData.get()) {
-        return fData->containsBitmaps();
-    }
-    SkFAIL("Unreachable");
-    return false;
-}
+static int32_t gPictureGenerationID = SK_InvalidGenID;  // This will be set at link time.
 
-// fRecord OK
 static int32_t next_picture_generation_id() {
-    static int32_t  gPictureGenerationID = 0;
-    // do a loop in case our global wraps around, as we never want to
-    // return a 0
+    // Loop in case our global wraps around.
     int32_t genID;
     do {
         genID = sk_atomic_inc(&gPictureGenerationID) + 1;
@@ -617,7 +505,6 @@ static int32_t next_picture_generation_id() {
     return genID;
 }
 
-// fRecord OK
 uint32_t SkPicture::uniqueID() const {
     if (SK_InvalidGenID == fUniqueID) {
         fUniqueID = next_picture_generation_id();
@@ -625,7 +512,6 @@ uint32_t SkPicture::uniqueID() const {
     return fUniqueID;
 }
 
-// fRecord OK
 SkPicture::SkPicture(SkScalar width, SkScalar height, SkRecord* record, SkBBoxHierarchy* bbh)
     : fCullWidth(width)
     , fCullHeight(height)
@@ -652,15 +538,3 @@ void SkPicture::callDeletionListeners() {
 
     fDeletionListeners.unrefAll();
 }
-
-// fRecord OK
-int SkPicture::approximateOpCount() const {
-    SkASSERT(fRecord.get() || fData.get());
-    if (fRecord.get()) {
-        return fRecord->count();
-    }
-    if (fData.get()) {
-        return fData->opCount();
-    }
-    return 0;
-}