Replace SkPicture(SkStream) constructors with a factory.

src/core/SkPicture.cpp

 
 /*
  * Copyright 2007 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 
 #include "SkPictureFlat.h"
@@ skipping 246 matching lines @@
     this->endRecording();
     if (fPlayback) {
         fPlayback->draw(*surface, callback);
     }
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 #include "SkStream.h"
 
-SkPicture::SkPicture(SkStream* stream) {
-    this->initFromStream(stream, NULL, NULL);
+bool SkPicture::StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) {
+    if (NULL == stream) {
+        return false;
+    }
+
+    SkPictInfo info;
+    if (!stream->read(&info, sizeof(SkPictInfo))) {
+        return false;
+    }
+    if (PICTURE_VERSION != info.fVersion) {
+        return false;
+    }
+    if (!stream->readBool()) {

[reed1, 2013/06/25 15:40:10]

// What is the bool we are checking?

[scroggo, 2013/06/25 16:54:35]

In SkPicture::serialize, we record true if there was a playback to write to the
stream (whether it was created by the call to serialize or it was on the picture
to begin with). If not, we record false.

I started to say that false meant there was nothing to draw, but you could have
a picture with a record or playback which is empty.

This actually changes the behavior in an unintentional way though. The intended
change in behavior is:

remove the pointer to success, and return NULL instead of an empty picture.

In addition, it includes the following change to behavior:

if there was no playback, return NULL (analogous to setting success to false in
the former case)

The latest patch retains the old behavior: i.e. a picture without a playback
will deserialize into an empty picture. That may be a good thing to reconsider,
though. Is there a benefit to deserializing an empty picture?

+        return false;
+    }
+
+    if (pInfo != NULL) {
+        *pInfo = info;
+    }
+    return true;
 }
 
-SkPicture::SkPicture(SkStream* stream, bool* success, InstallPixelRefProc proc) {
-    this->initFromStream(stream, success, proc);
-}
+SkPicture::SkPicture(SkPicturePlayback* playback, int width, int height)
+    : fPlayback(playback)
+    , fRecord(NULL)
+    , fWidth(width)
+    , fHeight(height) {}
 
-void SkPicture::initFromStream(SkStream* stream, bool* success, InstallPixelRefProc proc) {
-    if (success) {
-        *success = false;
-    }
-    fRecord = NULL;
-    fPlayback = NULL;
-    fWidth = fHeight = 0;
-
+SkPicture* SkPicture::CreateFromStream(SkStream* stream, InstallPixelRefProc proc) {
     SkPictInfo info;
 
-    if (!stream->read(&info, sizeof(info))) {
-        return;
-    }
-    if (PICTURE_VERSION != info.fVersion) {
-        return;
+    if (!StreamIsSKP(stream, &info)) {
+        return NULL;
     }
 
-    if (stream->readBool()) {
-        fPlayback = SkNEW_ARGS(SkPicturePlayback, (stream, info, proc));
-    }
+    SkPicturePlayback* playback = SkNEW_ARGS(SkPicturePlayback, (stream, info, proc));
 
-    // do this at the end, so that they will be zero if we hit an error.
-    fWidth = info.fWidth;
-    fHeight = info.fHeight;
-    if (success) {
-        *success = true;
-    }
+    return SkNEW_ARGS(SkPicture, (playback, info.fWidth, info.fHeight));
 }
 
 void SkPicture::serialize(SkWStream* stream, EncodeBitmap encoder) const {
     SkPicturePlayback* playback = fPlayback;
 
     if (NULL == playback && fRecord) {
         playback = SkNEW_ARGS(SkPicturePlayback, (*fRecord));
     }
 
     SkPictInfo info;
@@ skipping 23 matching lines @@
 }
 
 #ifdef SK_BUILD_FOR_ANDROID
 void SkPicture::abortPlayback() {
     if (NULL == fPlayback) {
         return;
     }
     fPlayback->abort();
 }
 #endif