Check for invalid SkPictures

- Remove hasRecorded() since nobody uses it.
- Add "success" boolean to SkPicture stream constructor
- Track failures in render_pictures and bench_pictures
Review URL: https://codereview.appspot.com/6493105

git-svn-id: http://skia.googlecode.com/svn/trunk@5573 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h
index 2b61c420323cbb1abd52f1bd5856eae5de9d18ef..00eadb83a6f30b41fd7ca7447014081a1d72b1b5 100644 (file)
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -38,10 +38,10 @@ public:
     */
     SkPicture(const SkPicture& src);
     /**
-     *  Recreate a picture that was serialized into a stream. If an error occurs
-     *  the picture will be "empty" : width and height == 0
+     *  Recreate a picture that was serialized into a stream. *success is set to
+     *  true if the picture was deserialized successfully and false otherwise.
      */
-    explicit SkPicture(SkStream*);
+    explicit SkPicture(SkStream*, bool* success = NULL);
     virtual ~SkPicture();
 
     /**
@@ -112,11 +112,6 @@ public:
     */
     void endRecording();
 
-    /** Returns true if any draw commands have been recorded since the last
-        call to beginRecording.
-    */
-    bool hasRecorded() const;
-
     /** Replays the drawing commands on the specified canvas. This internally
         calls endRecording() if that has not already been called.
         @param surface the canvas receiving the drawing commands.

diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index 8c453c668856196879be09b264dc21e5c30a7923..4c7ccf7c0fe3d735af87dce66326ee1014e7902b 100644 (file)
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -207,10 +207,6 @@ SkCanvas* SkPicture::beginRecording(int width, int height,
     return fRecord;
 }
 
-bool SkPicture::hasRecorded() const {
-    return NULL != fRecord && fRecord->writeStream().size() > 0;
-}
-
 SkCanvas* SkPicture::getRecordingCanvas() const {
     // will be null if we are not recording
     return fRecord;
@@ -246,7 +242,10 @@ void SkPicture::draw(SkCanvas* surface) {
 // V6 : added serialization of SkPath's bounds (and packed its flags tighter)
 #define PICTURE_VERSION     6
 
-SkPicture::SkPicture(SkStream* stream) : SkRefCnt() {
+SkPicture::SkPicture(SkStream* stream, bool* success) : SkRefCnt() {
+    if (success) {
+        *success = false;
+    }
     fRecord = NULL;
     fPlayback = NULL;
     fWidth = fHeight = 0;
@@ -273,6 +272,9 @@ SkPicture::SkPicture(SkStream* stream) : SkRefCnt() {
     // 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;
+    }
 }
 
 void SkPicture::serialize(SkWStream* stream) const {

diff --git a/tools/bench_pictures_main.cpp b/tools/bench_pictures_main.cpp
index 93b1cf9646743dcdb21dab92f7cfd3c101bc2654..d6837e9079cb2f3bcb1d6987296a3d1e5a1d91f4 100644 (file)
--- a/tools/bench_pictures_main.cpp
+++ b/tools/bench_pictures_main.cpp
@@ -97,7 +97,7 @@ static void usage(const char* argv0) {
 
 SkBenchLogger gLogger;
 
-static void run_single_benchmark(const SkString& inputPath,
+static bool run_single_benchmark(const SkString& inputPath,
                                  sk_tools::PictureBenchmark& benchmark) {
     SkFILEStream inputStream;
 
@@ -106,11 +106,18 @@ static void run_single_benchmark(const SkString& inputPath,
         SkString err;
         err.printf("Could not open file %s\n", inputPath.c_str());
         gLogger.logError(err);
-        return;
+        return false;
     }
 
-    SkPicture* picture = SkNEW_ARGS(SkPicture, (&inputStream));
+    bool success = false;
+    SkPicture* picture = SkNEW_ARGS(SkPicture, (&inputStream, &success));
     SkAutoTUnref<SkPicture> aur(picture);
+    if (!success) {
+        SkString err;
+        err.printf("Could not read an SkPicture from %s\n", inputPath.c_str());
+        gLogger.logError(err);
+        return false;
+    }
 
     SkString filename;
     sk_tools::get_basename(&filename, inputPath);
@@ -124,6 +131,7 @@ static void run_single_benchmark(const SkString& inputPath,
     sk_tools::resize_if_needed(&aur);
 
     benchmark.run(aur);
+    return true;
 }
 
 static void parse_commandline(int argc, char* const argv[], SkTArray<SkString>* inputs,
@@ -388,19 +396,23 @@ static void parse_commandline(int argc, char* const argv[], SkTArray<SkString>*
     gLogger.logProgress(commandLine);
 }
 
-static void process_input(const SkString& input, sk_tools::PictureBenchmark& benchmark) {
+static int process_input(const SkString& input,
+                         sk_tools::PictureBenchmark& benchmark) {
     SkOSFile::Iter iter(input.c_str(), "skp");
     SkString inputFilename;
-
+    int failures = 0;
     if (iter.next(&inputFilename)) {
         do {
             SkString inputPath;
             sk_tools::make_filepath(&inputPath, input, inputFilename);
-            run_single_benchmark(inputPath, benchmark);
+            if (!run_single_benchmark(inputPath, benchmark))
+                ++failures;
         } while(iter.next(&inputFilename));
     } else {
-          run_single_benchmark(input, benchmark);
+        if (!run_single_benchmark(input, benchmark))
+            ++failures;
     }
+    return failures;
 }
 
 int main(int argc, char* const argv[]) {
@@ -414,7 +426,15 @@ int main(int argc, char* const argv[]) {
 
     parse_commandline(argc, argv, &inputs, &benchmark);
 
+    int failures = 0;
     for (int i = 0; i < inputs.count(); ++i) {
-        process_input(inputs[i], benchmark);
+        failures += process_input(inputs[i], benchmark);
+    }
+
+    if (failures != 0) {
+        SkString err;
+        err.printf("Failed to run %i benchmarks.\n", failures);
+        gLogger.logError(err);
+        return 1;
     }
 }

diff --git a/tools/render_pictures_main.cpp b/tools/render_pictures_main.cpp
index 7d54516d8d5d10d3a49135975d7ed33a007a5112..5ad00729dd570601b0bb2659d45ee10b1c119f4f 100644 (file)
--- a/tools/render_pictures_main.cpp
+++ b/tools/render_pictures_main.cpp
@@ -79,7 +79,7 @@ static void make_output_filepath(SkString* path, const SkString& dir,
     path->append("png");
 }
 
-static void write_output(const SkString& outputDir, const SkString& inputFilename,
+static bool write_output(const SkString& outputDir, const SkString& inputFilename,
                          const sk_tools::PictureRenderer& renderer) {
     SkString outputPath;
     make_output_filepath(&outputPath, outputDir, inputFilename);
@@ -87,9 +87,10 @@ static void write_output(const SkString& outputDir, const SkString& inputFilenam
     if (!isWritten) {
         SkDebugf("Could not write to file %s\n", outputPath.c_str());
     }
+    return isWritten;
 }
 
-static void render_picture(const SkString& inputPath, const SkString& outputDir,
+static bool render_picture(const SkString& inputPath, const SkString& outputDir,
                            sk_tools::PictureRenderer& renderer) {
     SkString inputFilename;
     sk_tools::get_basename(&inputFilename, inputPath);
@@ -98,11 +99,16 @@ static void render_picture(const SkString& inputPath, const SkString& outputDir,
     inputStream.setPath(inputPath.c_str());
     if (!inputStream.isValid()) {
         SkDebugf("Could not open file %s\n", inputPath.c_str());
-        return;
+        return false;
     }
 
-    SkPicture* picture = SkNEW_ARGS(SkPicture, (&inputStream));
+    bool success = false;
+    SkPicture* picture = SkNEW_ARGS(SkPicture, (&inputStream, &success));
     SkAutoTUnref<SkPicture> aur(picture);
+    if (!success) {
+        SkDebugf("Could not read an SkPicture from %s\n", inputPath.c_str());
+        return false;
+    }
 
     SkDebugf("drawing... [%i %i] %s\n", picture->width(), picture->height(),
              inputPath.c_str());
@@ -116,26 +122,30 @@ static void render_picture(const SkString& inputPath, const SkString& outputDir,
 
     renderer.resetState();
 
-    write_output(outputDir, inputFilename, renderer);
+    success = write_output(outputDir, inputFilename, renderer);
 
     renderer.end();
+    return success;
 }
 
-static void process_input(const SkString& input, const SkString& outputDir,
+static int process_input(const SkString& input, const SkString& outputDir,
                           sk_tools::PictureRenderer& renderer) {
     SkOSFile::Iter iter(input.c_str(), "skp");
     SkString inputFilename;
-
+    int failures = 0;
     if (iter.next(&inputFilename)) {
         do {
             SkString inputPath;
             sk_tools::make_filepath(&inputPath, input, inputFilename);
-            render_picture(inputPath, outputDir, renderer);
+            if (!render_picture(inputPath, outputDir, renderer))
+              ++failures;
         } while(iter.next(&inputFilename));
     } else {
         SkString inputPath(input);
-        render_picture(inputPath, outputDir, renderer);
+        if (!render_picture(inputPath, outputDir, renderer))
+          ++failures;
     }
+    return failures;
 }
 
 static void parse_commandline(int argc, char* const argv[], SkTArray<SkString>* inputs,
@@ -282,7 +292,7 @@ static void parse_commandline(int argc, char* const argv[], SkTArray<SkString>*
 }
 
 int main(int argc, char* const argv[]) {
-    SkGraphics::Init();
+    SkAutoGraphics ag;
     SkTArray<SkString> inputs;
     sk_tools::PictureRenderer* renderer = NULL;
 
@@ -290,10 +300,14 @@ int main(int argc, char* const argv[]) {
     SkString outputDir = inputs[inputs.count() - 1];
     SkASSERT(renderer);
 
+    int failures = 0;
     for (int i = 0; i < inputs.count() - 1; i ++) {
-        process_input(inputs[i], outputDir, *renderer);
+        failures += process_input(inputs[i], outputDir, *renderer);
+    }
+    if (failures != 0) {
+        SkDebugf("Failed to render %i pictures.\n", failures);
+        return 1;
     }
-
 #if SK_SUPPORT_GPU
 #if GR_CACHE_STATS
     if (renderer->isUsingGpuDevice()) {
@@ -305,5 +319,4 @@ int main(int argc, char* const argv[]) {
 #endif
 
     SkDELETE(renderer);
-    SkGraphics::Term();
 }