add --writeChecksumBasedFilenames flag to render_pictures

tools/PictureRenderer.cpp

 /*
  * Copyright 2012 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "PictureRenderer.h"
 #include "picture_utils.h"
 #include "SamplePipeControllers.h"
@@ skipping 40 matching lines @@
 };
 
 /* TODO(epoger): These constants are already maintained in 2 other places:
  * gm/gm_json.py and gm/gm_expectations.cpp.  We shouldn't add yet a third place.
  * Figure out a way to share the definitions instead.
  */
 const static char kJsonKey_ActualResults[]   = "actual-results";
 const static char kJsonKey_ActualResults_NoComparison[]  = "no-comparison";
 const static char kJsonKey_Hashtype_Bitmap_64bitMD5[]  = "bitmap-64bitMD5";
 
-void ImageResultsSummary::add(const char *testName, const SkBitmap& bitmap) {
-    uint64_t hash;
-    SkAssertResult(SkBitmapHasher::ComputeDigest(bitmap, &hash));
+void ImageResultsSummary::add(const char *testName, uint64_t hash) {
     Json::Value jsonTypeValuePair;
     jsonTypeValuePair.append(Json::Value(kJsonKey_Hashtype_Bitmap_64bitMD5));
     jsonTypeValuePair.append(Json::UInt64(hash));
     fActualResultsNoComparison[testName] = jsonTypeValuePair;
 }
 
+void ImageResultsSummary::add(const char *testName, const SkBitmap& bitmap) {
+    uint64_t hash;
+    SkAssertResult(SkBitmapHasher::ComputeDigest(bitmap, &hash));

[robertphillips, 2014/03/19 14:42:36]

this->

[robertphillips1, 2014/03/19 14:44:15]

this->

[epoger, 2014/03/19 15:20:29]

Done.

+    add(testName, hash);
+}
+
 void ImageResultsSummary::writeToFile(const char *filename) {
     Json::Value actualResults;
     actualResults[kJsonKey_ActualResults_NoComparison] = fActualResultsNoComparison;
     Json::Value root;
     root[kJsonKey_ActualResults] = actualResults;
     std::string jsonStdString = root.toStyledString();
     SkFILEWStream stream(filename);
     stream.write(jsonStdString.c_str(), jsonStdString.length());
 }
 
-void PictureRenderer::init(SkPicture* pict) {
+void PictureRenderer::init(SkPicture* pict, const SkString& outputDir,
+                           const SkString& inputFilename, bool useChecksumBasedFilenames) {
+    fOutputDir.set(outputDir);
+    fInputFilename.set(inputFilename);
+    fUseChecksumBasedFilenames = useChecksumBasedFilenames;
+
     SkASSERT(NULL == fPicture);
     SkASSERT(NULL == fCanvas.get());
     if (fPicture != NULL || NULL != fCanvas.get()) {
         return;
     }
 
     SkASSERT(pict != NULL);
     if (NULL == pict) {
         return;
     }
@@ skipping 183 matching lines @@
 }
 
 uint32_t PictureRenderer::recordFlags() {
     return ((kNone_BBoxHierarchyType == fBBoxHierarchyType) ? 0 :
         SkPicture::kOptimizeForClippedPlayback_RecordingFlag) |
         SkPicture::kUsePathBoundsForClip_RecordingFlag;
 }
 
 /**
  * Write the canvas to the specified path.
+ *
  * @param canvas Must be non-null. Canvas to be written to a file.
- * @param path Path for the file to be written. Should have no extension; write() will append
- *             an appropriate one. Passed in by value so it can be modified.
+ * @param outputDir If nonempty, write the binary image to a file within this directory.
+ * @param inputFilename If we are writing out a binary image, use this to build its filename.
  * @param jsonSummaryPtr If not null, add image results to this summary.
+ * @param useChecksumBasedFilenames If true, use checksum-based filenames when writing to disk.
+ * @param numberToAppend If not null, append this number to the filename.
  * @return bool True if the Canvas is written to a file.
  *
  * TODO(epoger): Right now, all canvases must pass through this function in order to be appended
  * to the ImageResultsSummary.  We need some way to add bitmaps to the ImageResultsSummary
  * even if --writePath has not been specified (and thus this function is not called).
  *
  * One fix would be to pass in these path elements separately, and allow this function to be
  * called even if --writePath was not specified...
  *  const char *outputDir   // NULL if we don't want to write image files to disk
  *  const char *filename    // name we use within JSON summary, and as the filename within outputDir
+ *
+ * UPDATE: Now that outputDir and inputFilename are passed separately, we should be able to do that.
  */

[robertphillips1, 2014/03/19 14:44:15]

Write

[epoger, 2014/03/19 15:20:29]

I think the function name is already consistent with
https://sites.google.com/site/skiadocs/developer-documentation/contributing-c...
, right?

Static non-class functions in implementation files are lower case with
underscores separating words:

static inline bool tastes_like_chicken(Food food) {
    return kIceCream_Food != food;
}

[robertphillips, 2014/03/19 16:06:50]

Ah yes (thought it was class scoped for no good reason).

-static bool write(SkCanvas* canvas, const SkString* path, ImageResultsSummary *jsonSummaryPtr) {
+static bool write(SkCanvas* canvas, const SkString& outputDir, const SkString& inputFilename,
+                  ImageResultsSummary *jsonSummaryPtr, bool useChecksumBasedFilenames,
+                  const int* numberToAppend=NULL) {
     SkASSERT(canvas != NULL);
     if (NULL == canvas) {
         return false;
     }
 
-    SkASSERT(path != NULL);  // TODO(epoger): we want to remove this constraint, as noted above
-    SkString fullPathname(*path);
-    fullPathname.append(".png");
-
     SkBitmap bitmap;
     SkISize size = canvas->getDeviceSize();
     sk_tools::setup_bitmap(&bitmap, size.width(), size.height());
 
+    // Make sure we only compute the bitmap hash once (at most).
+    uint64_t hash;
+    bool generatedHash = false;
+
     canvas->readPixels(&bitmap, 0, 0);
     sk_tools::force_all_opaque(bitmap);
 
+    SkString outputFilename(inputFilename);
+    outputFilename.remove(outputFilename.size() - 4, 4);
+    if (NULL != numberToAppend) {
+        outputFilename.appendf("%i", *numberToAppend);
+    }
+    outputFilename.append(".png");
+    // TODO(epoger): what about including the config type within outputFilename?  That way,
+    // we could combine results of different config types without conflicting filenames.
+
     if (NULL != jsonSummaryPtr) {
-        // EPOGER: This is a hacky way of constructing the filename associated with the
-        // image checksum; we assume that outputDir is not NULL, and we remove outputDir
-        // from fullPathname.
-        //
-        // EPOGER: what about including the config type within hashFilename?  That way,
-        // we could combine results of different config types without conflicting filenames.
-        SkString hashFilename;
-        sk_tools::get_basename(&hashFilename, fullPathname);
-        jsonSummaryPtr->add(hashFilename.c_str(), bitmap);
+        if (!generatedHash) {
+            SkAssertResult(SkBitmapHasher::ComputeDigest(bitmap, &hash));
+            generatedHash = true;
+        }
+        jsonSummaryPtr->add(outputFilename.c_str(), hash);
     }
 
+    // Update outputFilename AFTER adding to JSON summary, but BEFORE writing out the image file.
+    if (useChecksumBasedFilenames) {
+        if (!generatedHash) {
+            SkAssertResult(SkBitmapHasher::ComputeDigest(bitmap, &hash));
+            generatedHash = true;
+        }
+        outputFilename.set(kJsonKey_Hashtype_Bitmap_64bitMD5);
+        outputFilename.append("_");
+        outputFilename.appendU64(hash);
+        outputFilename.append(".png");
+    }
+
+    SkASSERT(!outputDir.isEmpty()); // TODO(epoger): we want to remove this constraint,
+                                    // as noted above
+    SkString fullPathname;
+    make_filepath(&fullPathname, outputDir, outputFilename);
     return SkImageEncoder::EncodeFile(fullPathname.c_str(), bitmap, SkImageEncoder::kPNG_Type, 100);
 }
 
-/**
- * If path is non NULL, append number to it, and call write() to write the
- * provided canvas to a file. Returns true if path is NULL or if write() succeeds.
- */
-static bool writeAppendNumber(SkCanvas* canvas, const SkString* path, int number,
-                              ImageResultsSummary *jsonSummaryPtr) {
-    if (NULL == path) {
-        return true;
-    }
-    SkString pathWithNumber(*path);
-    pathWithNumber.appendf("%i", number);
-    return write(canvas, &pathWithNumber, jsonSummaryPtr);
-}
-
 ///////////////////////////////////////////////////////////////////////////////////////////////
 
 SkCanvas* RecordPictureRenderer::setupCanvas(int width, int height) {
     // defer the canvas setup until the render step
     return NULL;
 }
 
 // the size_t* parameter is deprecated, so we ignore it
 static SkData* encode_bitmap_to_data(size_t*, const SkBitmap& bm) {
     return SkImageEncoder::EncodeData(bm, SkImageEncoder::kPNG_Type, 100);
 }
 
-bool RecordPictureRenderer::render(const SkString* path, SkBitmap** out) {
+bool RecordPictureRenderer::render(SkBitmap** out) {
     SkAutoTUnref<SkPicture> replayer(this->createPicture());
     SkCanvas* recorder = replayer->beginRecording(this->getViewWidth(), this->getViewHeight(),
                                                   this->recordFlags());
     this->scaleToScaleFactor(recorder);
     fPicture->draw(recorder);
     replayer->endRecording();
-    if (path != NULL) {
+    if (!fOutputDir.isEmpty()) {
         // Record the new picture as a new SKP with PNG encoded bitmaps.
-        SkString skpPath(*path);
-        // ".skp" was removed from 'path' before being passed in here.
-        skpPath.append(".skp");
+        SkString skpPath;
+        make_filepath(&skpPath, fOutputDir, fInputFilename);
         SkFILEWStream stream(skpPath.c_str());
         replayer->serialize(&stream, &encode_bitmap_to_data);
         return true;
     }
     return false;
 }
 
 SkString RecordPictureRenderer::getConfigNameInternal() {
     return SkString("record");
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////
 
-bool PipePictureRenderer::render(const SkString* path, SkBitmap** out) {
+bool PipePictureRenderer::render(SkBitmap** out) {
     SkASSERT(fCanvas.get() != NULL);
     SkASSERT(fPicture != NULL);
     if (NULL == fCanvas.get() || NULL == fPicture) {
         return false;
     }
 
     PipeController pipeController(fCanvas.get());
     SkGPipeWriter writer;
     SkCanvas* pipeCanvas = writer.startRecording(&pipeController);
     pipeCanvas->drawPicture(*fPicture);
     writer.endRecording();
     fCanvas->flush();
-    if (NULL != path) {
-        return write(fCanvas, path, fJsonSummaryPtr);
+    if (!fOutputDir.isEmpty()) {
+        return write(fCanvas, fOutputDir, fInputFilename, fJsonSummaryPtr,
+                     fUseChecksumBasedFilenames);
     }
     if (NULL != out) {
         *out = SkNEW(SkBitmap);
         setup_bitmap(*out, fPicture->width(), fPicture->height());
         fCanvas->readPixels(*out, 0, 0);
     }
     return true;
 }
 
 SkString PipePictureRenderer::getConfigNameInternal() {
     return SkString("pipe");
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////
 
-void SimplePictureRenderer::init(SkPicture* picture) {
-    INHERITED::init(picture);
+void SimplePictureRenderer::init(SkPicture* picture, const SkString& outputDir,
+                                 const SkString& inputFilename, bool useChecksumBasedFilenames) {
+    INHERITED::init(picture, outputDir, inputFilename, useChecksumBasedFilenames);
     this->buildBBoxHierarchy();
 }
 
-bool SimplePictureRenderer::render(const SkString* path, SkBitmap** out) {
+bool SimplePictureRenderer::render(SkBitmap** out) {
     SkASSERT(fCanvas.get() != NULL);
     SkASSERT(fPicture != NULL);
     if (NULL == fCanvas.get() || NULL == fPicture) {
         return false;
     }
 
     fCanvas->drawPicture(*fPicture);
     fCanvas->flush();
-    if (NULL != path) {
-        return write(fCanvas, path, fJsonSummaryPtr);
+    if (!fOutputDir.isEmpty()) {
+        return write(fCanvas, fOutputDir, fInputFilename, fJsonSummaryPtr,
+                     fUseChecksumBasedFilenames);
     }
 
     if (NULL != out) {
         *out = SkNEW(SkBitmap);
         setup_bitmap(*out, fPicture->width(), fPicture->height());
         fCanvas->readPixels(*out, 0, 0);
     }
 
     return true;
 }
 
 SkString SimplePictureRenderer::getConfigNameInternal() {
     return SkString("simple");
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////
 
 TiledPictureRenderer::TiledPictureRenderer()
     : fTileWidth(kDefaultTileWidth)
     , fTileHeight(kDefaultTileHeight)
     , fTileWidthPercentage(0.0)
     , fTileHeightPercentage(0.0)
     , fTileMinPowerOf2Width(0)
     , fCurrentTileOffset(-1)
     , fTilesX(0)
     , fTilesY(0) { }
 
-void TiledPictureRenderer::init(SkPicture* pict) {
+void TiledPictureRenderer::init(SkPicture* pict, const SkString& outputDir,
+                                const SkString& inputFilename, bool useChecksumBasedFilenames) {
     SkASSERT(pict != NULL);
     SkASSERT(0 == fTileRects.count());
     if (NULL == pict || fTileRects.count() != 0) {
         return;
     }
 
     // Do not call INHERITED::init(), which would create a (potentially large) canvas which is not
     // used by bench_pictures.
     fPicture = pict;
+    fOutputDir.set(outputDir);
+    fInputFilename.set(inputFilename);
+    fUseChecksumBasedFilenames = useChecksumBasedFilenames;
     fPicture->ref();
     this->buildBBoxHierarchy();
 
     if (fTileWidthPercentage > 0) {
         fTileWidth = sk_float_ceil2int(float(fTileWidthPercentage * fPicture->width() / 100));
     }
     if (fTileHeightPercentage > 0) {
         fTileHeight = sk_float_ceil2int(float(fTileHeightPercentage * fPicture->height() / 100));
     }
 
@@ skipping 141 matching lines @@
         return true;
     }
     return false;
 }
 
 void TiledPictureRenderer::drawCurrentTile() {
     SkASSERT(fCurrentTileOffset >= 0 && fCurrentTileOffset < fTileRects.count());
     draw_tile_to_canvas(fCanvas, fTileRects[fCurrentTileOffset], fPicture);
 }
 
-bool TiledPictureRenderer::render(const SkString* path, SkBitmap** out) {
+bool TiledPictureRenderer::render(SkBitmap** out) {
     SkASSERT(fPicture != NULL);
     if (NULL == fPicture) {
         return false;
     }
 
     SkBitmap bitmap;
     if (out){
         *out = SkNEW(SkBitmap);
         setup_bitmap(*out, fPicture->width(), fPicture->height());
         setup_bitmap(&bitmap, fTileWidth, fTileHeight);
     }
     bool success = true;
     for (int i = 0; i < fTileRects.count(); ++i) {
         draw_tile_to_canvas(fCanvas, fTileRects[i], fPicture);
-        if (NULL != path) {
-            success &= writeAppendNumber(fCanvas, path, i, fJsonSummaryPtr);
+        if (!fOutputDir.isEmpty()) {
+            success &= write(fCanvas, fOutputDir, fInputFilename, fJsonSummaryPtr,
+                             fUseChecksumBasedFilenames, &i);
         }
         if (NULL != out) {
             if (fCanvas->readPixels(&bitmap, 0, 0)) {
                 // Add this tile to the entire bitmap.
                 bitmapCopyAtOffset(bitmap, *out, SkScalarFloorToInt(fTileRects[i].left()),
                                    SkScalarFloorToInt(fTileRects[i].top()));
             } else {
                 success = false;
             }
         }
@@ skipping 38 matching lines @@
     return name;
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////
 
 // Holds all of the information needed to draw a set of tiles.
 class CloneData : public SkRunnable {
 
 public:
     CloneData(SkPicture* clone, SkCanvas* canvas, SkTDArray<SkRect>& rects, int start, int end,
-              SkRunnable* done, ImageResultsSummary* jsonSummaryPtr)
+              SkRunnable* done, ImageResultsSummary* jsonSummaryPtr, bool useChecksumBasedFilenames)
         : fClone(clone)
         , fCanvas(canvas)
-        , fPath(NULL)
         , fRects(rects)
         , fStart(start)
         , fEnd(end)
         , fSuccess(NULL)
         , fDone(done)
-        , fJsonSummaryPtr(jsonSummaryPtr) {
+        , fJsonSummaryPtr(jsonSummaryPtr)
+        , fUseChecksumBasedFilenames(useChecksumBasedFilenames) {
         SkASSERT(fDone != NULL);
     }
 
     virtual void run() SK_OVERRIDE {
         SkGraphics::SetTLSFontCacheLimit(1024 * 1024);
 
         SkBitmap bitmap;
         if (fBitmap != NULL) {
             // All tiles are the same size.
             setup_bitmap(&bitmap, SkScalarFloorToInt(fRects[0].width()), SkScalarFloorToInt(fRects[0].height()));
         }
 
         for (int i = fStart; i < fEnd; i++) {
             draw_tile_to_canvas(fCanvas, fRects[i], fClone);
-            if ((fPath != NULL) && !writeAppendNumber(fCanvas, fPath, i, fJsonSummaryPtr)
+            if ((!fOutputDir.isEmpty())
+                && !write(fCanvas, fOutputDir, fInputFilename, fJsonSummaryPtr,
+                          fUseChecksumBasedFilenames, &i)
                 && fSuccess != NULL) {
                 *fSuccess = false;
                 // If one tile fails to write to a file, do not continue drawing the rest.
                 break;
             }
             if (fBitmap != NULL) {
                 if (fCanvas->readPixels(&bitmap, 0, 0)) {
                     SkAutoLockPixels alp(*fBitmap);
                     bitmapCopyAtOffset(bitmap, fBitmap, SkScalarFloorToInt(fRects[i].left()),
                                        SkScalarFloorToInt(fRects[i].top()));
                 } else {
                     *fSuccess = false;
                     // If one tile fails to read pixels, do not continue drawing the rest.
                     break;
                 }
             }
         }
         fDone->run();
     }
 
-    void setPathAndSuccess(const SkString* path, bool* success) {
-        fPath = path;
+    void setPathsAndSuccess(const SkString& outputDir, const SkString& inputFilename,
+                            bool* success) {
+        fOutputDir.set(outputDir);
+        fInputFilename.set(inputFilename);
         fSuccess = success;
     }
 
     void setBitmap(SkBitmap* bitmap) {
         fBitmap = bitmap;
     }
 
 private:
     // All pointers unowned.
     SkPicture*         fClone;      // Picture to draw from. Each CloneData has a unique one which
                                     // is threadsafe.
     SkCanvas*          fCanvas;     // Canvas to draw to. Reused for each tile.
-    const SkString*    fPath;       // If non-null, path to write the result to as a PNG.
+    SkString           fOutputDir;  // If not empty, write results into this directory.
+    SkString           fInputFilename; // Filename of input SkPicture file.
     SkTDArray<SkRect>& fRects;      // All tiles of the picture.
     const int          fStart;      // Range of tiles drawn by this thread.
     const int          fEnd;
     bool*              fSuccess;    // Only meaningful if path is non-null. Shared by all threads,
                                     // and only set to false upon failure to write to a PNG.
     SkRunnable*        fDone;
     SkBitmap*          fBitmap;
     ImageResultsSummary* fJsonSummaryPtr;
+    bool               fUseChecksumBasedFilenames;
 };
 
 MultiCorePictureRenderer::MultiCorePictureRenderer(int threadCount)
 : fNumThreads(threadCount)
 , fThreadPool(threadCount)
 , fCountdown(threadCount) {
     // Only need to create fNumThreads - 1 clones, since one thread will use the base
     // picture.
     fPictureClones = SkNEW_ARRAY(SkPicture, fNumThreads - 1);
     fCloneData = SkNEW_ARRAY(CloneData*, fNumThreads);
 }
 
-void MultiCorePictureRenderer::init(SkPicture *pict) {
+void MultiCorePictureRenderer::init(SkPicture *pict, const SkString& outputDir,
+                                    const SkString& inputFilename, bool useChecksumBasedFilenames) {
     // Set fPicture and the tiles.
-    this->INHERITED::init(pict);
+    this->INHERITED::init(pict, outputDir, inputFilename, useChecksumBasedFilenames);
     for (int i = 0; i < fNumThreads; ++i) {
         *fCanvasPool.append() = this->setupCanvas(this->getTileWidth(), this->getTileHeight());
     }
     // Only need to create fNumThreads - 1 clones, since one thread will use the base picture.
     fPicture->clone(fPictureClones, fNumThreads - 1);
     // Populate each thread with the appropriate data.
     // Group the tiles into nearly equal size chunks, rounding up so we're sure to cover them all.
     const int chunkSize = (fTileRects.count() + fNumThreads - 1) / fNumThreads;
 
     for (int i = 0; i < fNumThreads; i++) {
         SkPicture* pic;
         if (i == fNumThreads-1) {
             // The last set will use the original SkPicture.
             pic = fPicture;
         } else {
             pic = &fPictureClones[i];
         }
         const int start = i * chunkSize;
         const int end = SkMin32(start + chunkSize, fTileRects.count());
         fCloneData[i] = SkNEW_ARGS(CloneData,
                                    (pic, fCanvasPool[i], fTileRects, start, end, &fCountdown,
-                                    fJsonSummaryPtr));
+                                    fJsonSummaryPtr, useChecksumBasedFilenames));
     }
 }
 
-bool MultiCorePictureRenderer::render(const SkString *path, SkBitmap** out) {
+bool MultiCorePictureRenderer::render(SkBitmap** out) {
     bool success = true;
-    if (path != NULL) {
+    if (!fOutputDir.isEmpty()) {
         for (int i = 0; i < fNumThreads-1; i++) {
-            fCloneData[i]->setPathAndSuccess(path, &success);
+            fCloneData[i]->setPathsAndSuccess(fOutputDir, fInputFilename, &success);
         }
     }
 
     if (NULL != out) {
         *out = SkNEW(SkBitmap);
         setup_bitmap(*out, fPicture->width(), fPicture->height());
         for (int i = 0; i < fNumThreads; i++) {
             fCloneData[i]->setBitmap(*out);
         }
     } else {
@@ skipping 37 matching lines @@
 ///////////////////////////////////////////////////////////////////////////////////////////////
 
 void PlaybackCreationRenderer::setup() {
     fReplayer.reset(this->createPicture());
     SkCanvas* recorder = fReplayer->beginRecording(this->getViewWidth(), this->getViewHeight(),
                                                    this->recordFlags());
     this->scaleToScaleFactor(recorder);
     fPicture->draw(recorder);
 }
 
-bool PlaybackCreationRenderer::render(const SkString*, SkBitmap** out) {
+bool PlaybackCreationRenderer::render(SkBitmap** out) {
     fReplayer->endRecording();
     // Since this class does not actually render, return false.
     return false;
 }
 
 SkString PlaybackCreationRenderer::getConfigNameInternal() {
     return SkString("playback_creation");
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////
@@ skipping 26 matching lines @@
                 fPicture->height(), fGridInfo));
     }
     SkASSERT(0); // invalid bbhType
     return NULL;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 class GatherRenderer : public PictureRenderer {
 public:
-    virtual bool render(const SkString* path, SkBitmap** out = NULL)
+    virtual bool render(SkBitmap** out = NULL)
             SK_OVERRIDE {
         SkRect bounds = SkRect::MakeWH(SkIntToScalar(fPicture->width()),
                                        SkIntToScalar(fPicture->height()));
         SkData* data = SkPictureUtils::GatherPixelRefs(fPicture, bounds);
         SkSafeUnref(data);
 
-        return NULL == path;    // we don't have anything to write
+        return (fOutputDir.isEmpty());    // we don't have anything to write
     }
 
 private:
     virtual SkString getConfigNameInternal() SK_OVERRIDE {
         return SkString("gather_pixelrefs");
     }
 };
 
 PictureRenderer* CreateGatherPixelRefsRenderer() {
     return SkNEW(GatherRenderer);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 class PictureCloneRenderer : public PictureRenderer {
 public:
-    virtual bool render(const SkString* path, SkBitmap** out = NULL)
+    virtual bool render(SkBitmap** out = NULL)
             SK_OVERRIDE {
         for (int i = 0; i < 100; ++i) {
             SkPicture* clone = fPicture->clone();
             SkSafeUnref(clone);
         }
 
-        return NULL == path;    // we don't have anything to write
+        return (fOutputDir.isEmpty());    // we don't have anything to write
     }
 
 private:
     virtual SkString getConfigNameInternal() SK_OVERRIDE {
         return SkString("picture_clone");
     }
 };
 
 PictureRenderer* CreatePictureCloneRenderer() {
     return SkNEW(PictureCloneRenderer);
 }
 
 } // namespace sk_tools