Replace SkPicture(SkStream) constructors with a factory.
SkPicture:
Remove the constructors which take an SkStream as an argument. Rather
than having to check a variable for success, the factory will return
NULL on failure.
Add a protected function for determining if an SkStream is an SKP
to share code with SkTimedPicture.
In the factory, check for a NULL SkStream.
Use a default decoder (from BUG:
https://code.google.com/p/skia/issues/detail?id=1325)
SkDebuggerGUI:
Call SkPicture::CreateFromStream when necessary.
Write a factory for creating SkTimedPictures and use it.
Use the factory throughout tools.
Add include/lazy to utils and effects gyp include_dirs so SkPicture.h
can reference SkImageDecoder.h which references SkBitmapFactory.h (in
include/lazy).
Changes code Chromium uses, so this will require a temporary Skia
and then a change to Chromium to use the new Skia code.
TODO: Create a decoder that does nothing to be used by pinspect,
lua pictures, etc, and allow it to not assert in SkOrderedReadBuffer.
R=reed@google.com
Review URL: https://codereview.chromium.org/17113004
git-svn-id: http://skia.googlecode.com/svn/trunk@9822 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/debugger/QT/SkDebuggerGUI.cpp b/debugger/QT/SkDebuggerGUI.cpp
index f5f9bc6..6727707 100644
--- a/debugger/QT/SkDebuggerGUI.cpp
+++ b/debugger/QT/SkDebuggerGUI.cpp
@@ -237,35 +237,24 @@
// Wrap SkPicture to allow installation of an SkTimedPicturePlayback object
class SkTimedPicture : public SkPicture {
public:
- explicit SkTimedPicture(SkStream* stream, bool* success, SkPicture::InstallPixelRefProc proc,
- const SkTDArray<bool>& deletedCommands) {
- if (success) {
- *success = false;
- }
- fRecord = NULL;
- fPlayback = NULL;
- fWidth = fHeight = 0;
-
+ static SkTimedPicture* CreateTimedPicture(SkStream* stream,
+ SkPicture::InstallPixelRefProc proc,
+ const SkTDArray<bool>& deletedCommands) {
SkPictInfo info;
-
- if (!stream->read(&info, sizeof(info))) {
- return;
- }
- if (SkPicture::PICTURE_VERSION != info.fVersion) {
- return;
+ if (!StreamIsSKP(stream, &info)) {
+ return NULL;
}
+ SkTimedPicturePlayback* playback;
+ // Check to see if there is a playback to recreate.
if (stream->readBool()) {
- fPlayback = SkNEW_ARGS(SkTimedPicturePlayback,
- (stream, info, proc, deletedCommands));
+ playback = SkNEW_ARGS(SkTimedPicturePlayback,
+ (stream, info, proc, deletedCommands));
+ } else {
+ playback = NULL;
}
- // 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(SkTimedPicture, (playback, info.fWidth, info.fHeight));
}
void resetTimes() { ((SkTimedPicturePlayback*) fPlayback)->resetTimes(); }
@@ -282,6 +271,9 @@
private:
// disallow default ctor b.c. we don't have a good way to setup the fPlayback ptr
SkTimedPicture();
+ // Private ctor only used by CreateTimedPicture, which has created the playback.
+ SkTimedPicture(SkTimedPicturePlayback* playback, int width, int height)
+ : INHERITED(playback, width, height) {}
// disallow the copy ctor - enabling would require copying code from SkPicture
SkTimedPicture(const SkTimedPicture& src);
@@ -338,10 +330,9 @@
return;
}
- bool success = false;
- SkTimedPicture picture(&inputStream, &success, &SkImageDecoder::DecodeMemory,
- fSkipCommands);
- if (!success) {
+ SkAutoTUnref<SkTimedPicture> picture(SkTimedPicture::CreateTimedPicture(&inputStream,
+ &SkImageDecoder::DecodeMemory, fSkipCommands));
+ if (NULL == picture.get()) {
return;
}
@@ -377,20 +368,20 @@
static const int kNumRepeats = 10;
- run(&picture, renderer, kNumRepeats);
+ run(picture.get(), renderer, kNumRepeats);
- SkASSERT(picture.count() == fListWidget.count());
+ SkASSERT(picture->count() == fListWidget.count());
// extract the individual command times from the SkTimedPlaybackPicture
- for (int i = 0; i < picture.count(); ++i) {
- double temp = picture.time(i);
+ for (int i = 0; i < picture->count(); ++i) {
+ double temp = picture->time(i);
QListWidgetItem* item = fListWidget.item(i);
item->setData(Qt::UserRole + 4, 100.0*temp);
}
- setupOverviewText(picture.typeTimes(), picture.totTime(), kNumRepeats);
+ setupOverviewText(picture->typeTimes(), picture->totTime(), kNumRepeats);
}
void SkDebuggerGUI::actionCancel() {
@@ -905,12 +896,9 @@
fLoading = true;
SkStream* stream = SkNEW_ARGS(SkFILEStream, (fileName.c_str()));
- bool success = false;
+ SkPicture* picture = SkPicture::CreateFromStream(stream);
- SkPicture* picture = SkNEW_ARGS(SkPicture,
- (stream, &success, &SkImageDecoder::DecodeMemory));
-
- if (!success) {
+ if (NULL == picture) {
QMessageBox::critical(this, "Error loading file", "Couldn't read file, sorry.");
SkSafeUnref(stream);
return;
diff --git a/gm/gmmain.cpp b/gm/gmmain.cpp
index ad7d55f..8c015e7 100644
--- a/gm/gmmain.cpp
+++ b/gm/gmmain.cpp
@@ -1025,10 +1025,7 @@
//@todo thudson 22 April 2011 when can we safely delete [] dst?
storage.copyTo(dst);
SkMemoryStream pictReadback(dst, streamSize);
- bool success;
- // Pass a decoding bitmap function so that the factory GM (which has an SkBitmap with
- // encoded data) does not fail.
- SkPicture* retval = new SkPicture (&pictReadback, &success, &SkImageDecoder::DecodeMemory);
+ SkPicture* retval = SkPicture::CreateFromStream(&pictReadback);
return retval;
}
diff --git a/gyp/effects.gyp b/gyp/effects.gyp
index 91458eb..d69c820 100644
--- a/gyp/effects.gyp
+++ b/gyp/effects.gyp
@@ -1,3 +1,4 @@
+# Gyp file for effects
{
'targets': [
{
@@ -12,6 +13,7 @@
'../include/config',
'../include/core',
'../include/effects',
+ '../include/lazy',
'../include/utils',
'../src/core',
],
diff --git a/gyp/utils.gyp b/gyp/utils.gyp
index 4ac9284..1b51b18 100644
--- a/gyp/utils.gyp
+++ b/gyp/utils.gyp
@@ -1,3 +1,4 @@
+# Gyp for utils.
{
'targets': [
{
@@ -10,6 +11,7 @@
'../include/core',
'../include/effects',
'../include/images',
+ '../include/lazy',
'../include/pipe',
'../include/utils',
'../include/utils/mac',
diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h
index d01cadc..0bac3f7 100644
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -11,6 +11,7 @@
#define SkPicture_DEFINED
#include "SkBitmap.h"
+#include "SkImageDecoder.h"
#include "SkRefCnt.h"
class SkBBoxHierarchy;
@@ -22,6 +23,8 @@
class SkStream;
class SkWStream;
+struct SkPictInfo;
+
/** \class SkPicture
The SkPicture class records the drawing commands made to a canvas, to
@@ -42,13 +45,6 @@
SkPicture(const SkPicture& src);
/**
- * Recreate a picture that was serialized into a stream.
- * On failure, silently creates an empty picture.
- * @param SkStream Serialized picture data.
- */
- explicit SkPicture(SkStream*);
-
- /**
* Function signature defining a function that sets up an SkBitmap from encoded data. On
* success, the SkBitmap should have its Config, width, height, rowBytes and pixelref set.
* If the installed pixelref has decoded the data into pixels, then the src buffer need not be
@@ -64,12 +60,13 @@
/**
* Recreate a picture that was serialized into a stream.
* @param SkStream Serialized picture data.
- * @param success Output parameter. If non-NULL, will be set to true if the picture was
- * deserialized successfully and false otherwise.
* @param proc Function pointer for installing pixelrefs on SkBitmaps representing the
* encoded bitmap data from the stream.
+ * @return A new SkPicture representing the serialized data, or NULL if the stream is
+ * invalid.
*/
- SkPicture(SkStream*, bool* success, InstallPixelRefProc proc);
+ static SkPicture* CreateFromStream(SkStream*,
+ InstallPixelRefProc proc = &SkImageDecoder::DecodeMemory);
virtual ~SkPicture();
@@ -220,13 +217,20 @@
SkPictureRecord* fRecord;
int fWidth, fHeight;
+ // Create a new SkPicture from an existing SkPicturePlayback. Ref count of
+ // playback is unchanged.
+ SkPicture(SkPicturePlayback*, int width, int height);
+
// For testing. Derived classes may instantiate an alternate
// SkBBoxHierarchy implementation
virtual SkBBoxHierarchy* createBBoxHierarchy() const;
+ // Return true if the SkStream represents a serialized picture, and fills out
+ // SkPictInfo. After this function returns, the SkStream is not rewound; it
+ // will be ready to be parsed to create an SkPicturePlayback.
+ // If false is returned, SkPictInfo is unmodified.
+ static bool StreamIsSKP(SkStream*, SkPictInfo*);
private:
- void initFromStream(SkStream*, bool* success, InstallPixelRefProc);
-
friend class SkFlatPicture;
friend class SkPicturePlayback;
diff --git a/samplecode/SampleApp.cpp b/samplecode/SampleApp.cpp
index 380511f..3825197 100644
--- a/samplecode/SampleApp.cpp
+++ b/samplecode/SampleApp.cpp
@@ -1379,8 +1379,10 @@
SkAutoDataUnref data(ostream.copyToData());
SkMemoryStream istream(data->data(), data->size());
- SkPicture pict(&istream);
- orig->drawPicture(pict);
+ SkAutoTUnref<SkPicture> pict(SkPicture::CreateFromStream(&istream));
+ if (pict.get() != NULL) {
+ orig->drawPicture(*pict.get());
+ }
} else {
fPicture->draw(orig);
fPicture->unref();
diff --git a/samplecode/SamplePictFile.cpp b/samplecode/SamplePictFile.cpp
index 847894c..867118d 100644
--- a/samplecode/SamplePictFile.cpp
+++ b/samplecode/SamplePictFile.cpp
@@ -48,7 +48,7 @@
} else {
SkFILEStream stream(path);
if (stream.isValid()) {
- pic = SkNEW_ARGS(SkPicture, (&stream, NULL, &SkImageDecoder::DecodeMemory));
+ pic = SkPicture::CreateFromStream(&stream);
} else {
SkDebugf("coun't load picture at \"path\"\n", path);
}
diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index ab2faea..2b488de 100644
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -264,41 +264,47 @@
#include "SkStream.h"
-SkPicture::SkPicture(SkStream* stream) {
- this->initFromStream(stream, NULL, NULL);
-}
-
-SkPicture::SkPicture(SkStream* stream, bool* success, InstallPixelRefProc proc) {
- this->initFromStream(stream, success, proc);
-}
-
-void SkPicture::initFromStream(SkStream* stream, bool* success, InstallPixelRefProc proc) {
- if (success) {
- *success = false;
+bool SkPicture::StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) {
+ if (NULL == stream) {
+ return false;
}
- fRecord = NULL;
- fPlayback = NULL;
- fWidth = fHeight = 0;
SkPictInfo info;
-
- if (!stream->read(&info, sizeof(info))) {
- return;
+ if (!stream->read(&info, sizeof(SkPictInfo))) {
+ return false;
}
if (PICTURE_VERSION != info.fVersion) {
- return;
+ return false;
}
+ if (pInfo != NULL) {
+ *pInfo = info;
+ }
+ return true;
+}
+
+SkPicture::SkPicture(SkPicturePlayback* playback, int width, int height)
+ : fPlayback(playback)
+ , fRecord(NULL)
+ , fWidth(width)
+ , fHeight(height) {}
+
+SkPicture* SkPicture::CreateFromStream(SkStream* stream, InstallPixelRefProc proc) {
+ SkPictInfo info;
+
+ if (!StreamIsSKP(stream, &info)) {
+ return NULL;
+ }
+
+ SkPicturePlayback* playback;
+ // Check to see if there is a playback to recreate.
if (stream->readBool()) {
- fPlayback = SkNEW_ARGS(SkPicturePlayback, (stream, info, proc));
+ playback = SkNEW_ARGS(SkPicturePlayback, (stream, info, proc));
+ } else {
+ playback = NULL;
}
- // 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 {
diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index 898d379..fe1a037 100644
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -520,15 +520,14 @@
case PICT_PICTURE_TAG: {
fPictureCount = size;
fPictureRefs = SkNEW_ARRAY(SkPicture*, fPictureCount);
- bool success;
for (int i = 0; i < fPictureCount; i++) {
- fPictureRefs[i] = SkNEW_ARGS(SkPicture, (stream, &success, proc));
- // Success can only be false if PICTURE_VERSION does not match
+ fPictureRefs[i] = SkPicture::CreateFromStream(stream, proc);
+ // CreateFromStream can only fail 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);
+ SkASSERT(fPictureRefs[i] != NULL);
}
} break;
case PICT_BUFFER_SIZE_TAG: {
diff --git a/tests/PictureTest.cpp b/tests/PictureTest.cpp
index 23ff689..49bc57b 100644
--- a/tests/PictureTest.cpp
+++ b/tests/PictureTest.cpp
@@ -420,10 +420,9 @@
context.fReporter = reporter;
SkSetErrorCallback(assert_one_parse_error_cb, &context);
SkMemoryStream pictureStream(picture1);
- bool success;
SkClearLastError();
- SkPicture pictureFromStream(&pictureStream, &success, NULL);
- REPORTER_ASSERT(reporter, success);
+ SkAutoUnref pictureFromStream(SkPicture::CreateFromStream(&pictureStream, NULL));
+ REPORTER_ASSERT(reporter, pictureFromStream.get() != NULL);
SkClearLastError();
SkSetErrorCallback(NULL, NULL);
}
diff --git a/tools/bench_pictures_main.cpp b/tools/bench_pictures_main.cpp
index 46d97de..720621e 100644
--- a/tools/bench_pictures_main.cpp
+++ b/tools/bench_pictures_main.cpp
@@ -173,16 +173,15 @@
gLruImageCache.setImageCacheLimit(0);
}
- bool success = false;
- SkPicture* picture;
+ SkPicture::InstallPixelRefProc proc;
if (FLAGS_deferImageDecoding) {
- picture = SkNEW_ARGS(SkPicture, (&inputStream, &success, &lazy_decode_bitmap));
+ proc = &lazy_decode_bitmap;
} else {
- picture = SkNEW_ARGS(SkPicture, (&inputStream, &success, &SkImageDecoder::DecodeMemory));
+ proc = &SkImageDecoder::DecodeMemory;
}
- SkAutoTDelete<SkPicture> ad(picture);
+ SkAutoTUnref<SkPicture> picture(SkPicture::CreateFromStream(&inputStream, proc));
- if (!success) {
+ if (NULL == picture.get()) {
SkString err;
err.printf("Could not read an SkPicture from %s\n", inputPath.c_str());
gLogger.logError(err);
diff --git a/tools/filtermain.cpp b/tools/filtermain.cpp
index 4114a9d..39c484d 100644
--- a/tools/filtermain.cpp
+++ b/tools/filtermain.cpp
@@ -666,7 +666,7 @@
SkFILEStream inStream(inFile.c_str());
if (inStream.isValid()) {
- inPicture.reset(SkNEW_ARGS(SkPicture, (&inStream, NULL, &SkImageDecoder::DecodeMemory)));
+ inPicture.reset(SkPicture::CreateFromStream(&inStream));
}
if (NULL == inPicture.get()) {
diff --git a/tools/lua/lua_pictures.cpp b/tools/lua/lua_pictures.cpp
index 02b9a57..f1bca28 100644
--- a/tools/lua/lua_pictures.cpp
+++ b/tools/lua/lua_pictures.cpp
@@ -46,13 +46,7 @@
SkAutoTUnref<SkStream> stream(SkStream::NewFromFile(path));
SkPicture* pic = NULL;
if (stream.get()) {
- bool success;
- pic = SkNEW_ARGS(SkPicture, (stream.get(), &success,
- &lazy_decode_bitmap));
- if (!success) {
- SkDELETE(pic);
- pic = NULL;
- }
+ pic = SkPicture::CreateFromStream(stream.get(), &lazy_decode_bitmap);
}
return pic;
}
diff --git a/tools/pinspect.cpp b/tools/pinspect.cpp
index f6304e6..969d87c 100644
--- a/tools/pinspect.cpp
+++ b/tools/pinspect.cpp
@@ -40,11 +40,10 @@
}
stream.rewind();
- bool success = false;
- SkPicture* pic = SkNEW_ARGS(SkPicture, (&stream, &success, &lazy_decode_bitmap));
- if (!success) {
+ SkPicture* pic = SkPicture::CreateFromStream(&stream, &lazy_decode_bitmap);
+ if (NULL == pic) {
SkDebugf("Could not create SkPicture: %s\n", path);
- return pic;
+ return NULL;
}
printf("picture size:[%d %d]\n", pic->width(), pic->height());
return pic;
diff --git a/tools/render_pdfs_main.cpp b/tools/render_pdfs_main.cpp
index 1821548..f443502 100644
--- a/tools/render_pdfs_main.cpp
+++ b/tools/render_pdfs_main.cpp
@@ -9,7 +9,6 @@
#include "SkDevice.h"
#include "SkForceLinking.h"
#include "SkGraphics.h"
-#include "SkImageDecoder.h"
#include "SkImageEncoder.h"
#include "SkOSFile.h"
#include "SkPicture.h"
@@ -169,11 +168,9 @@
return false;
}
- bool success = false;
- SkAutoTUnref<SkPicture>
- picture(SkNEW_ARGS(SkPicture, (&inputStream, &success, &SkImageDecoder::DecodeMemory)));
+ SkAutoTUnref<SkPicture> picture(SkPicture::CreateFromStream(&inputStream));
- if (!success) {
+ if (NULL == picture.get()) {
SkDebugf("Could not read an SkPicture from %s\n", inputPath.c_str());
return false;
}
@@ -185,7 +182,7 @@
renderer.render();
- success = write_output(outputDir, inputFilename, renderer);
+ bool success = write_output(outputDir, inputFilename, renderer);
renderer.end();
return success;
diff --git a/tools/render_pictures_main.cpp b/tools/render_pictures_main.cpp
index 4a7e708..de477d3 100644
--- a/tools/render_pictures_main.cpp
+++ b/tools/render_pictures_main.cpp
@@ -149,21 +149,22 @@
return false;
}
- SkDebugf("deserializing... %s\n", inputPath.c_str());
-
- bool success = false;
- SkPicture* picture;
+ SkPicture::InstallPixelRefProc proc;
if (FLAGS_deferImageDecoding) {
- picture = SkNEW_ARGS(SkPicture, (&inputStream, &success, &lazy_decode_bitmap));
+ proc = &lazy_decode_bitmap;
} else if (FLAGS_writeEncodedImages) {
SkASSERT(!FLAGS_writePath.isEmpty());
reset_image_file_base_name(inputFilename);
- picture = SkNEW_ARGS(SkPicture, (&inputStream, &success, &write_image_to_file));
+ proc = &write_image_to_file;
} else {
- picture = SkNEW_ARGS(SkPicture, (&inputStream, &success, &SkImageDecoder::DecodeMemory));
+ proc = &SkImageDecoder::DecodeMemory;
}
- if (!success) {
+ SkDebugf("deserializing... %s\n", inputPath.c_str());
+
+ SkPicture* picture = SkPicture::CreateFromStream(&inputStream, proc);
+
+ if (NULL == picture) {
SkDebugf("Could not read an SkPicture from %s\n", inputPath.c_str());
return false;
}
@@ -186,7 +187,7 @@
make_output_filepath(outputPath, *outputDir, inputFilename);
}
- success = renderer.render(outputPath, out);
+ bool success = renderer.render(outputPath, out);
if (outputPath) {
if (!success) {
SkDebugf("Could not write to file %s\n", outputPath->c_str());