diff options
author | 2014-03-19 17:26:07 +0000 | |
---|---|---|
committer | 2014-03-19 17:26:07 +0000 | |
commit | f5e315ccf1ae2941f7cf53fa53e5c8c4bb665fe1 (patch) | |
tree | 4bada82865131be20404e6e1f230a708a324bcc5 /tools | |
parent | fffb2cd4639076b799a68cc0d1fc04d376b1ac3d (diff) |
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
Diffstat (limited to 'tools')
-rw-r--r-- | tools/CopyTilesRenderer.cpp | 20 | ||||
-rw-r--r-- | tools/CopyTilesRenderer.h | 5 | ||||
-rw-r--r-- | tools/PictureBenchmark.cpp | 2 | ||||
-rw-r--r-- | tools/PictureRenderer.cpp | 183 | ||||
-rw-r--r-- | tools/PictureRenderer.h | 71 | ||||
-rw-r--r-- | tools/bbh_shootout.cpp | 6 | ||||
-rw-r--r-- | tools/render_pictures_main.cpp | 27 | ||||
-rwxr-xr-x | tools/tests/render_pictures_test.py | 103 |
8 files changed, 303 insertions, 114 deletions
diff --git a/tools/CopyTilesRenderer.cpp b/tools/CopyTilesRenderer.cpp index 1298d43f0f..341d93edae 100644 --- a/tools/CopyTilesRenderer.cpp +++ b/tools/CopyTilesRenderer.cpp @@ -20,11 +20,17 @@ namespace sk_tools { : 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 @@ namespace sk_tools { 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 @@ namespace sk_tools { 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 9a8b3745bd..ef1ccb09b4 100644 --- a/tools/CopyTilesRenderer.h +++ b/tools/CopyTilesRenderer.h @@ -23,13 +23,14 @@ namespace sk_tools { 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 6d8cada09e..3cc34157fd 100644 --- a/tools/PictureBenchmark.cpp +++ b/tools/PictureBenchmark.cpp @@ -73,7 +73,7 @@ void PictureBenchmark::run(SkPicture* pict) { 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 466e31d28e..f62a05d846 100644 --- a/tools/PictureRenderer.cpp +++ b/tools/PictureRenderer.cpp @@ -58,15 +58,19 @@ 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)); + this->add(testName, hash); +} + void ImageResultsSummary::writeToFile(const char *filename) { Json::Value actualResults; actualResults[kJsonKey_ActualResults_NoComparison] = fActualResultsNoComparison; @@ -77,7 +81,12 @@ void ImageResultsSummary::writeToFile(const char *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) { + 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 @@ void PictureRenderer::init(SkPicture* pict) { 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 @@ uint32_t PictureRenderer::recordFlags() { /** * 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,51 +315,62 @@ uint32_t PictureRenderer::recordFlags() { * 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); - 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); + 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. - return SkImageEncoder::EncodeFile(fullPathname.c_str(), bitmap, SkImageEncoder::kPNG_Type, 100); -} + if (NULL != jsonSummaryPtr) { + if (!generatedHash) { + SkAssertResult(SkBitmapHasher::ComputeDigest(bitmap, &hash)); + generatedHash = true; + } + jsonSummaryPtr->add(outputFilename.c_str(), hash); + } -/** - * 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; + // 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"); } - SkString pathWithNumber(*path); - pathWithNumber.appendf("%i", number); - return write(canvas, &pathWithNumber, jsonSummaryPtr); + + 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); } /////////////////////////////////////////////////////////////////////////////////////////////// @@ -354,18 +385,17 @@ 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; @@ -379,7 +409,7 @@ SkString RecordPictureRenderer::getConfigNameInternal() { /////////////////////////////////////////////////////////////////////////////////////////////// -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 @@ bool PipePictureRenderer::render(const SkString* path, SkBitmap** out) { 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 @@ SkString PipePictureRenderer::getConfigNameInternal() { /////////////////////////////////////////////////////////////////////////////////////////////// -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 @@ bool SimplePictureRenderer::render(const SkString* path, SkBitmap** out) { 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 @@ TiledPictureRenderer::TiledPictureRenderer() , 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 @@ void TiledPictureRenderer::init(SkPicture* pict) { // 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 @@ void TiledPictureRenderer::drawCurrentTile() { 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 TiledPictureRenderer::render(const SkString* path, SkBitmap** out) { 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 @@ 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); } @@ -722,7 +760,9 @@ public: 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 @@ public: 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 @@ private: 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 @@ private: SkRunnable* fDone; SkBitmap* fBitmap; ImageResultsSummary* fJsonSummaryPtr; + bool fUseChecksumBasedFilenames; }; MultiCorePictureRenderer::MultiCorePictureRenderer(int threadCount) @@ -778,9 +822,10 @@ MultiCorePictureRenderer::MultiCorePictureRenderer(int threadCount) 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 @@ void MultiCorePictureRenderer::init(SkPicture *pict) { 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 @@ void PlaybackCreationRenderer::setup() { 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 @@ SkPicture* PictureRenderer::createPicture() { 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 @@ PictureRenderer* CreateGatherPixelRefsRenderer() { 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 68374dfe41..8192cacc0d 100644 --- a/tools/PictureRenderer.h +++ b/tools/PictureRenderer.h @@ -44,6 +44,14 @@ class TiledPictureRenderer; 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 @@ public: /** * 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 @@ public: 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 @@ public: 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 @@ protected: 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 @@ private: * 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 @@ private: 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 @@ private: 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 @@ class TiledPictureRenderer : public PictureRenderer { 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 @@ public: ~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 @@ class PlaybackCreationRenderer : public PictureRenderer { 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 8d9ab63b6f..65d37826d1 100644 --- a/tools/bbh_shootout.cpp +++ b/tools/bbh_shootout.cpp @@ -67,16 +67,16 @@ static void do_benchmark_work(sk_tools::PictureRenderer* renderer, 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 a8c393f985..4e51bfa66e 100644 --- a/tools/render_pictures_main.cpp +++ b/tools/render_pictures_main.cpp @@ -29,6 +29,8 @@ DEFINE_int32(maxComponentDiff, 256, "Maximum diff on a component, 0 - 256. Compo "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 @@ static bool render_picture_internal(const SkString& inputPath, const SkString* o 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 @@ static bool render_picture_internal(const SkString& inputPath, const SkString* o 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 @@ static bool render_picture_internal(const SkString& inputPath, const SkString* o 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 @@ static bool render_picture(const SkString& inputPath, const SkString* outputDir, 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 fa530d5f8b..162531a6ad 100755 --- a/tools/tests/render_pictures_test.py +++ b/tools/tests/render_pictures_test.py @@ -37,10 +37,15 @@ class RenderPicturesTest(base_unittest.TestCase): """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 @@ class RenderPicturesTest(base_unittest.TestCase): } } 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 @@ class RenderPicturesTest(base_unittest.TestCase): } } 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 @@ class RenderPicturesTest(base_unittest.TestCase): } } 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 @@ class RenderPicturesTest(base_unittest.TestCase): '--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. |