add --writeChecksumBasedFilenames flag to render_pictures
BUG=skia:1455,skia:2230
R=robertphillips@google.com, robertphillips@chromium.org
Author: epoger@google.com
Review URL: https://codereview.chromium.org/202983003
git-svn-id: http://skia.googlecode.com/svn/trunk@13859 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/debugger/QT/SkDebuggerGUI.cpp b/debugger/QT/SkDebuggerGUI.cpp
index b777aa1..619aa0a 100644
--- a/debugger/QT/SkDebuggerGUI.cpp
+++ b/debugger/QT/SkDebuggerGUI.cpp
@@ -319,10 +319,10 @@
return;
}
- renderer->init(pict);
+ renderer->init(pict, NULL, NULL, false);
renderer->setup();
- renderer->render(NULL);
+ renderer->render();
renderer->resetState(true); // flush, swapBuffers and Finish
// We throw this away the first batch of times to remove first time effects (such as paging in this program)
@@ -330,7 +330,7 @@
for (int i = 0; i < repeats; ++i) {
renderer->setup();
- renderer->render(NULL);
+ renderer->render();
renderer->resetState(false); // flush & swapBuffers, but don't Finish
}
renderer->resetState(true); // flush, swapBuffers and Finish
diff --git a/tools/CopyTilesRenderer.cpp b/tools/CopyTilesRenderer.cpp
index 1298d43..341d93e 100644
--- a/tools/CopyTilesRenderer.cpp
+++ b/tools/CopyTilesRenderer.cpp
@@ -20,11 +20,17 @@
: fXTilesPerLargeTile(x)
, fYTilesPerLargeTile(y) {
}
- void CopyTilesRenderer::init(SkPicture* pict) {
+ void CopyTilesRenderer::init(SkPicture* pict, const SkString* outputDir,
+ const SkString* inputFilename, bool useChecksumBasedFilenames) {
+ // Do not call INHERITED::init(), which would create a (potentially large) canvas which is
+ // not used by bench_pictures.
SkASSERT(pict != NULL);
// Only work with absolute widths (as opposed to percentages).
SkASSERT(this->getTileWidth() != 0 && this->getTileHeight() != 0);
fPicture = pict;
+ this->CopyString(&fOutputDir, outputDir);
+ this->CopyString(&fInputFilename, inputFilename);
+ fUseChecksumBasedFilenames = useChecksumBasedFilenames;
fPicture->ref();
this->buildBBoxHierarchy();
// In order to avoid allocating a large canvas (particularly important for GPU), create one
@@ -34,7 +40,7 @@
fCanvas.reset(this->INHERITED::setupCanvas(fLargeTileWidth, fLargeTileHeight));
}
- bool CopyTilesRenderer::render(const SkString* path, SkBitmap** out) {
+ bool CopyTilesRenderer::render(SkBitmap** out) {
int i = 0;
bool success = true;
SkBitmap dst;
@@ -59,10 +65,14 @@
SkDEBUGCODE(bool extracted =)
baseBitmap.extractSubset(&dst, subset);
SkASSERT(extracted);
- if (path != NULL) {
- // Similar to writeAppendNumber in PictureRenderer.cpp, but just encodes
+ if (!fOutputDir.isEmpty()) {
+ // Similar to write() in PictureRenderer.cpp, but just encodes
// a bitmap directly.
- SkString pathWithNumber(*path);
+ // TODO: Share more common code with write() to do this, to properly
+ // write out the JSON summary, etc.
+ SkString pathWithNumber;
+ make_filepath(&pathWithNumber, fOutputDir, fInputFilename);
+ pathWithNumber.remove(pathWithNumber.size() - 4, 4);
pathWithNumber.appendf("%i.png", i++);
SkBitmap copy;
#if SK_SUPPORT_GPU
diff --git a/tools/CopyTilesRenderer.h b/tools/CopyTilesRenderer.h
index 9a8b374..ef1ccb0 100644
--- a/tools/CopyTilesRenderer.h
+++ b/tools/CopyTilesRenderer.h
@@ -23,13 +23,14 @@
public:
CopyTilesRenderer(int x, int y);
- virtual void init(SkPicture* pict) SK_OVERRIDE;
+ virtual void init(SkPicture* pict, const SkString* outputDir, const SkString* inputFilename,
+ bool useChecksumBasedFilenames) SK_OVERRIDE;
/**
* Similar to TiledPictureRenderer, this will draw a PNG for each tile. However, the
* numbering (and actual tiles) will be different.
*/
- virtual bool render(const SkString* path, SkBitmap** out) SK_OVERRIDE;
+ virtual bool render(SkBitmap** out) SK_OVERRIDE;
virtual bool supportsTimingIndividualTiles() SK_OVERRIDE { return false; }
diff --git a/tools/PictureBenchmark.cpp b/tools/PictureBenchmark.cpp
index 6d8cada..3cc3415 100644
--- a/tools/PictureBenchmark.cpp
+++ b/tools/PictureBenchmark.cpp
@@ -73,7 +73,7 @@
return;
}
- fRenderer->init(pict);
+ fRenderer->init(pict, NULL, NULL, false);
// We throw this away to remove first time effects (such as paging in this program)
fRenderer->setup();
diff --git a/tools/PictureRenderer.cpp b/tools/PictureRenderer.cpp
index 466e31d..f62a05d 100644
--- a/tools/PictureRenderer.cpp
+++ b/tools/PictureRenderer.cpp
@@ -58,15 +58,19 @@
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));
+ this->add(testName, hash);
+}
+
void ImageResultsSummary::writeToFile(const char *filename) {
Json::Value actualResults;
actualResults[kJsonKey_ActualResults_NoComparison] = fActualResultsNoComparison;
@@ -77,7 +81,12 @@
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) {
+ this->CopyString(&fOutputDir, outputDir);
+ this->CopyString(&fInputFilename, inputFilename);
+ fUseChecksumBasedFilenames = useChecksumBasedFilenames;
+
SkASSERT(NULL == fPicture);
SkASSERT(NULL == fCanvas.get());
if (fPicture != NULL || NULL != fCanvas.get()) {
@@ -94,6 +103,14 @@
fCanvas.reset(this->setupCanvas());
}
+void PictureRenderer::CopyString(SkString* dest, const SkString* src) {
+ if (NULL != src) {
+ dest->set(*src);
+ } else {
+ dest->reset();
+ }
+}
+
class FlagsDrawFilter : public SkDrawFilter {
public:
FlagsDrawFilter(PictureRenderer::DrawFilterFlags* flags) :
@@ -281,10 +298,13 @@
/**
* 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
@@ -295,53 +315,64 @@
* 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.
*/
-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) {
@@ -354,18 +385,17 @@
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;
@@ -379,7 +409,7 @@
///////////////////////////////////////////////////////////////////////////////////////////////
-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) {
@@ -392,8 +422,9 @@
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);
@@ -409,12 +440,13 @@
///////////////////////////////////////////////////////////////////////////////////////////////
-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) {
@@ -423,8 +455,9 @@
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) {
@@ -452,7 +485,8 @@
, 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) {
@@ -462,6 +496,9 @@
// Do not call INHERITED::init(), which would create a (potentially large) canvas which is not
// used by bench_pictures.
fPicture = pict;
+ this->CopyString(&fOutputDir, outputDir);
+ this->CopyString(&fInputFilename, inputFilename);
+ fUseChecksumBasedFilenames = useChecksumBasedFilenames;
fPicture->ref();
this->buildBBoxHierarchy();
@@ -623,7 +660,7 @@
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;
@@ -638,8 +675,9 @@
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)) {
@@ -698,16 +736,16 @@
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);
}
@@ -722,7 +760,9 @@
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.
@@ -743,8 +783,10 @@
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;
}
@@ -757,7 +799,8 @@
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;
@@ -766,6 +809,7 @@
SkRunnable* fDone;
SkBitmap* fBitmap;
ImageResultsSummary* fJsonSummaryPtr;
+ bool fUseChecksumBasedFilenames;
};
MultiCorePictureRenderer::MultiCorePictureRenderer(int threadCount)
@@ -778,9 +822,10 @@
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());
}
@@ -802,15 +847,15 @@
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);
}
}
@@ -868,7 +913,7 @@
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;
@@ -915,14 +960,14 @@
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:
@@ -939,14 +984,14 @@
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:
diff --git a/tools/PictureRenderer.h b/tools/PictureRenderer.h
index 68374df..8192cac 100644
--- a/tools/PictureRenderer.h
+++ b/tools/PictureRenderer.h
@@ -44,6 +44,14 @@
class ImageResultsSummary {
public:
/**
+ * Adds this bitmap hash to the summary of results.
+ *
+ * @param testName name of the test
+ * @param hash hash to store
+ */
+ void add(const char *testName, uint64_t hash);
+
+ /**
* Adds this bitmap's hash to the summary of results.
*
* @param testName name of the test
@@ -105,8 +113,16 @@
/**
* Called with each new SkPicture to render.
+ *
+ * @param pict The SkPicture to render.
+ * @param outputDir The output directory within which this renderer should write files,
+ * or NULL if this renderer should not write files at all.
+ * @param inputFilename The name of the input file we are rendering.
+ * @param useChecksumBasedFilenames Whether to use checksum-based filenames when writing
+ * bitmap images to disk.
*/
- virtual void init(SkPicture* pict);
+ virtual void init(SkPicture* pict, const SkString* outputDir,
+ const SkString* inputFilename, bool useChecksumBasedFilenames);
/**
* Set the viewport so that only the portion listed gets drawn.
@@ -125,15 +141,20 @@
virtual void setup() {}
/**
- * Perform work that is to be timed. Typically this is rendering, but is also used for recording
- * and preparing picture for playback by the subclasses which do those.
- * If path is non-null, subclass implementations should call write().
- * @param path If non-null, also write the output to the file specified by path. path should
- * have no extension; it will be added by write().
- * @return bool True if rendering succeeded and, if path is non-null, the output was
- * successfully written to a file.
+ * Perform the work. If this is being called within the context of bench_pictures,
+ * this is the step that will be timed.
+ *
+ * Typically "the work" is rendering an SkPicture into a bitmap, but in some subclasses
+ * it is recording the source SkPicture into another SkPicture.
+ *
+ * If fOutputDir has been specified, the result of the work will be written to that dir.
+ *
+ * @param out If non-null, the implementing subclass MAY allocate an SkBitmap, copy the
+ * output image into it, and return it here. (Some subclasses ignore this parameter)
+ * @return bool True if rendering succeeded and, if fOutputDir had been specified, the output
+ * was successfully written to a file.
*/
- virtual bool render(const SkString* path, SkBitmap** out = NULL) = 0;
+ virtual bool render(SkBitmap** out = NULL) = 0;
/**
* Called once finished with a particular SkPicture, before calling init again, and before
@@ -371,11 +392,14 @@
protected:
SkAutoTUnref<SkCanvas> fCanvas;
SkPicture* fPicture;
+ bool fUseChecksumBasedFilenames;
ImageResultsSummary* fJsonSummaryPtr;
SkDeviceTypes fDeviceType;
BBoxHierarchyType fBBoxHierarchyType;
DrawFilterFlags fDrawFilters[SkDrawFilter::kTypeCount];
SkString fDrawFiltersConfig;
+ SkString fOutputDir;
+ SkString fInputFilename;
SkTileGridPicture::TileGridInfo fGridInfo; // used when fBBoxHierarchyType is TileGrid
void buildBBoxHierarchy();
@@ -402,6 +426,11 @@
SkCanvas* setupCanvas();
virtual SkCanvas* setupCanvas(int width, int height);
+ /**
+ * Copy src to dest; if src==NULL, set dest to empty string.
+ */
+ static void CopyString(SkString* dest, const SkString* src);
+
private:
SkISize fViewport;
SkScalar fScaleFactor;
@@ -421,7 +450,7 @@
* to time.
*/
class RecordPictureRenderer : public PictureRenderer {
- virtual bool render(const SkString*, SkBitmap** out = NULL) SK_OVERRIDE;
+ virtual bool render(SkBitmap** out = NULL) SK_OVERRIDE;
virtual SkString getPerIterTimeFormat() SK_OVERRIDE { return SkString("%.4f"); }
@@ -436,7 +465,7 @@
class PipePictureRenderer : public PictureRenderer {
public:
- virtual bool render(const SkString*, SkBitmap** out = NULL) SK_OVERRIDE;
+ virtual bool render(SkBitmap** out = NULL) SK_OVERRIDE;
private:
virtual SkString getConfigNameInternal() SK_OVERRIDE;
@@ -446,9 +475,10 @@
class SimplePictureRenderer : public PictureRenderer {
public:
- virtual void init(SkPicture* pict) SK_OVERRIDE;
+ virtual void init(SkPicture* pict, const SkString* outputDir,
+ const SkString* inputFilename, bool useChecksumBasedFilenames) SK_OVERRIDE;
- virtual bool render(const SkString*, SkBitmap** out = NULL) SK_OVERRIDE;
+ virtual bool render(SkBitmap** out = NULL) SK_OVERRIDE;
private:
virtual SkString getConfigNameInternal() SK_OVERRIDE;
@@ -460,14 +490,16 @@
public:
TiledPictureRenderer();
- virtual void init(SkPicture* pict) SK_OVERRIDE;
+ virtual void init(SkPicture* pict, const SkString* outputDir,
+ const SkString* inputFilename, bool useChecksumBasedFilenames) SK_OVERRIDE;
/**
- * Renders to tiles, rather than a single canvas. If a path is provided, a separate file is
+ * Renders to tiles, rather than a single canvas.
+ * If fOutputDir was provided, a separate file is
* created for each tile, named "path0.png", "path1.png", etc.
* Multithreaded mode currently does not support writing to a file.
*/
- virtual bool render(const SkString* path, SkBitmap** out = NULL) SK_OVERRIDE;
+ virtual bool render(SkBitmap** out = NULL) SK_OVERRIDE;
virtual void end() SK_OVERRIDE;
@@ -583,12 +615,13 @@
~MultiCorePictureRenderer();
- virtual void init(SkPicture* pict) SK_OVERRIDE;
+ virtual void init(SkPicture* pict, const SkString* outputDir,
+ const SkString* inputFilename, bool useChecksumBasedFilenames) SK_OVERRIDE;
/**
* Behaves like TiledPictureRenderer::render(), only using multiple threads.
*/
- virtual bool render(const SkString* path, SkBitmap** out = NULL) SK_OVERRIDE;
+ virtual bool render(SkBitmap** out = NULL) SK_OVERRIDE;
virtual void end() SK_OVERRIDE;
@@ -615,7 +648,7 @@
public:
virtual void setup() SK_OVERRIDE;
- virtual bool render(const SkString*, SkBitmap** out = NULL) SK_OVERRIDE;
+ virtual bool render(SkBitmap** out = NULL) SK_OVERRIDE;
virtual SkString getPerIterTimeFormat() SK_OVERRIDE { return SkString("%.4f"); }
diff --git a/tools/bbh_shootout.cpp b/tools/bbh_shootout.cpp
index 8d9ab63..65d3782 100644
--- a/tools/bbh_shootout.cpp
+++ b/tools/bbh_shootout.cpp
@@ -67,16 +67,16 @@
BenchTimer* timer) {
renderer->setBBoxHierarchyType(bBoxType);
renderer->setGridSize(FLAGS_tilesize, FLAGS_tilesize);
- renderer->init(pic);
+ renderer->init(pic, NULL, NULL, false);
SkDebugf("%s %d times...\n", renderer->getConfigName().c_str(), numRepeats);
for (int i = 0; i < numRepeats; ++i) {
renderer->setup();
// Render once to fill caches
- renderer->render(NULL, NULL);
+ renderer->render();
// Render again to measure
timer->start();
- renderer->render(NULL);
+ renderer->render();
timer->end();
}
}
diff --git a/tools/render_pictures_main.cpp b/tools/render_pictures_main.cpp
index a8c393f..4e51bfa 100644
--- a/tools/render_pictures_main.cpp
+++ b/tools/render_pictures_main.cpp
@@ -29,6 +29,8 @@
"by more than this amount are considered errors, though all diffs are reported. "
"Requires --validate.");
DECLARE_string(readPath);
+DEFINE_bool(writeChecksumBasedFilenames, false,
+ "When writing out images, use checksum-based filenames.");
DEFINE_bool(writeEncodedImages, false, "Any time the skp contains an encoded image, write it to a "
"file rather than decoding it. Requires writePath to be set. Skips drawing the full "
"skp to a file. Not compatible with deferImageDecoding.");
@@ -145,6 +147,10 @@
SkBitmap** out) {
SkString inputFilename;
sk_tools::get_basename(&inputFilename, inputPath);
+ SkString outputDirString;
+ if (NULL != outputDir && outputDir->size() > 0 && !FLAGS_writeEncodedImages) {
+ outputDirString.set(*outputDir);
+ }
SkFILEStream inputStream;
inputStream.setPath(inputPath.c_str());
@@ -189,7 +195,7 @@
SkDebugf("drawing... [%i %i] %s\n", picture->width(), picture->height(),
inputPath.c_str());
- renderer.init(picture);
+ renderer.init(picture, &outputDirString, &inputFilename, FLAGS_writeChecksumBasedFilenames);
if (FLAGS_preprocess) {
if (NULL != renderer.getCanvas()) {
@@ -199,18 +205,9 @@
renderer.setup();
- SkString* outputPath = NULL;
- if (NULL != outputDir && outputDir->size() > 0 && !FLAGS_writeEncodedImages) {
- outputPath = SkNEW(SkString);
- make_output_filepath(outputPath, *outputDir, inputFilename);
- }
-
- bool success = renderer.render(outputPath, out);
- if (outputPath) {
- if (!success) {
- SkDebugf("Could not write to file %s\n", outputPath->c_str());
- }
- SkDELETE(outputPath);
+ bool success = renderer.render(out);
+ if (!success) {
+ SkDebugf("Failed to render %s\n", inputFilename.c_str());
}
renderer.end();
@@ -352,13 +349,13 @@
sk_tools::force_all_opaque(*bitmap);
if (NULL != jsonSummaryPtr) {
- // EPOGER: This is a hacky way of constructing the filename associated with the
+ // TODO(epoger): This is a hacky way of constructing the filename associated with the
// image checksum; we basically are repeating the logic of make_output_filepath()
// and code below here, within here.
// It would be better for the filename (without outputDir) to be passed in here,
// and used both for the checksum file and writing into outputDir.
//
- // EPOGER: what about including the config type within hashFilename? That way,
+ // TODO(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, inputPath);
diff --git a/tools/tests/render_pictures_test.py b/tools/tests/render_pictures_test.py
index fa530d5..162531a 100755
--- a/tools/tests/render_pictures_test.py
+++ b/tools/tests/render_pictures_test.py
@@ -37,10 +37,15 @@
"""Run render_pictures with tiles and --writeWholeImage flag."""
output_json_path = os.path.join(self._temp_dir, 'output.json')
self._generate_skps()
+ # TODO(epoger): I noticed that when this is run without --writePath being
+ # specified, this test writes red.png and green.png to the current working
+ # directory. We should fix that... if --writePath is not specified, this
+ # probably shouldn't write out red.png and green.png at all!
self._run_render_pictures(['-r', self._input_skp_dir,
'--bbh', 'grid', '256', '256',
'--mode', 'tile', '256', '256',
'--writeJsonSummaryPath', output_json_path,
+ '--writePath', self._temp_dir,
'--writeWholeImage'])
expected_summary_dict = {
"actual-results" : {
@@ -53,6 +58,8 @@
}
}
self._assert_json_contents(output_json_path, expected_summary_dict)
+ self._assert_directory_contents(
+ self._temp_dir, ['red.png', 'green.png', 'output.json'])
def test_untiled(self):
"""Run without tiles."""
@@ -72,6 +79,32 @@
}
}
self._assert_json_contents(output_json_path, expected_summary_dict)
+ self._assert_directory_contents(
+ self._temp_dir, ['red.png', 'green.png', 'output.json'])
+
+ def test_untiled_writeChecksumBasedFilenames(self):
+ """Same as test_untiled, but with --writeChecksumBasedFilenames."""
+ output_json_path = os.path.join(self._temp_dir, 'output.json')
+ self._generate_skps()
+ self._run_render_pictures(['-r', self._input_skp_dir,
+ '--writeChecksumBasedFilenames',
+ '--writePath', self._temp_dir,
+ '--writeJsonSummaryPath', output_json_path])
+ expected_summary_dict = {
+ "actual-results" : {
+ "no-comparison" : {
+ # Manually verified: 640x400 red rectangle with black border
+ "red.png" : ["bitmap-64bitMD5", 11092453015575919668],
+ # Manually verified: 640x400 green rectangle with black border
+ "green.png" : ["bitmap-64bitMD5", 8891695120562235492],
+ }
+ }
+ }
+ self._assert_json_contents(output_json_path, expected_summary_dict)
+ self._assert_directory_contents(
+ self._temp_dir, ['bitmap-64bitMD5_11092453015575919668.png',
+ 'bitmap-64bitMD5_8891695120562235492.png',
+ 'output.json'])
def test_untiled_validate(self):
"""Same as test_untiled, but with --validate.
@@ -142,6 +175,62 @@
}
}
self._assert_json_contents(output_json_path, expected_summary_dict)
+ self._assert_directory_contents(
+ self._temp_dir,
+ ['red0.png', 'red1.png', 'red2.png', 'red3.png', 'red4.png', 'red5.png',
+ 'green0.png', 'green1.png', 'green2.png', 'green3.png', 'green4.png',
+ 'green5.png', 'output.json'])
+
+ def test_tiled_writeChecksumBasedFilenames(self):
+ """Same as test_tiled, but with --writeChecksumBasedFilenames."""
+ output_json_path = os.path.join(self._temp_dir, 'output.json')
+ self._generate_skps()
+ self._run_render_pictures(['-r', self._input_skp_dir,
+ '--bbh', 'grid', '256', '256',
+ '--mode', 'tile', '256', '256',
+ '--writeChecksumBasedFilenames',
+ '--writePath', self._temp_dir,
+ '--writeJsonSummaryPath', output_json_path])
+ expected_summary_dict = {
+ "actual-results" : {
+ "no-comparison" : {
+ # Manually verified these 6 images, all 256x256 tiles,
+ # consistent with a tiled version of the 640x400 red rect
+ # with black borders.
+ "red0.png" : ["bitmap-64bitMD5", 5815827069051002745],
+ "red1.png" : ["bitmap-64bitMD5", 9323613075234140270],
+ "red2.png" : ["bitmap-64bitMD5", 16670399404877552232],
+ "red3.png" : ["bitmap-64bitMD5", 2507897274083364964],
+ "red4.png" : ["bitmap-64bitMD5", 7325267995523877959],
+ "red5.png" : ["bitmap-64bitMD5", 2181381724594493116],
+ # Manually verified these 6 images, all 256x256 tiles,
+ # consistent with a tiled version of the 640x400 green rect
+ # with black borders.
+ "green0.png" : ["bitmap-64bitMD5", 12587324416545178013],
+ "green1.png" : ["bitmap-64bitMD5", 7624374914829746293],
+ "green2.png" : ["bitmap-64bitMD5", 5686489729535631913],
+ "green3.png" : ["bitmap-64bitMD5", 7980646035555096146],
+ "green4.png" : ["bitmap-64bitMD5", 17817086664365875131],
+ "green5.png" : ["bitmap-64bitMD5", 10673669813016809363],
+ }
+ }
+ }
+ self._assert_json_contents(output_json_path, expected_summary_dict)
+ self._assert_directory_contents(
+ self._temp_dir,
+ ['bitmap-64bitMD5_5815827069051002745.png',
+ 'bitmap-64bitMD5_9323613075234140270.png',
+ 'bitmap-64bitMD5_16670399404877552232.png',
+ 'bitmap-64bitMD5_2507897274083364964.png',
+ 'bitmap-64bitMD5_7325267995523877959.png',
+ 'bitmap-64bitMD5_2181381724594493116.png',
+ 'bitmap-64bitMD5_12587324416545178013.png',
+ 'bitmap-64bitMD5_7624374914829746293.png',
+ 'bitmap-64bitMD5_5686489729535631913.png',
+ 'bitmap-64bitMD5_7980646035555096146.png',
+ 'bitmap-64bitMD5_17817086664365875131.png',
+ 'bitmap-64bitMD5_10673669813016809363.png',
+ 'output.json'])
def _run_render_pictures(self, args):
binary = self.find_path_to_program('render_pictures')
@@ -179,6 +268,20 @@
'--writePath', str(output_path),
])
+ def _assert_directory_contents(self, dir_path, expected_filenames):
+ """Asserts that files found in a dir are identical to expected_filenames.
+
+ Args:
+ dir_path: Path to a directory on local disk.
+ expected_filenames: Set containing the expected filenames within the dir.
+
+ Raises:
+ AssertionError: contents of the directory are not identical to
+ expected_filenames.
+ """
+ self.assertEqual(set(os.listdir(dir_path)), set(expected_filenames))
+
+
def _assert_json_contents(self, json_path, expected_dict):
"""Asserts that contents of a JSON file are identical to expected_dict.