render_pictures: add --writeJsonSummaryPath
Known issues:
- JSON summary will be empty if --writePath not specified also
- testNames within summary do not include the config (8888, gpu, etc.)
BUG=
R=rmistry@google.com
Author: epoger@google.com
Review URL: https://codereview.chromium.org/98643007
git-svn-id: http://skia.googlecode.com/svn/trunk@12680 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/gyp/tools.gyp b/gyp/tools.gyp
index 04d4e3f..fd381b7 100644
--- a/gyp/tools.gyp
+++ b/gyp/tools.gyp
@@ -141,8 +141,8 @@
'../tools/skhello.cpp',
],
'dependencies': [
- 'pdf.gyp:pdf',
'flags.gyp:flags',
+ 'pdf.gyp:pdf',
],
}],
],
@@ -158,10 +158,10 @@
'../src/utils/',
],
'dependencies': [
- 'skia_lib.gyp:skia_lib',
'flags.gyp:flags',
'gm.gyp:gm_expectations',
'jsoncpp.gyp:jsoncpp',
+ 'skia_lib.gyp:skia_lib',
'utils.gyp:utils',
],
},
@@ -174,13 +174,13 @@
'../src/utils/SkLua.cpp',
],
'dependencies': [
- 'skia_lib.gyp:skia_lib',
'effects.gyp:effects',
- 'utils.gyp:utils',
'images.gyp:images',
+ 'lua.gyp:lua',
'pdf.gyp:pdf',
'ports.gyp:ports',
- 'lua.gyp:lua',
+ 'skia_lib.gyp:skia_lib',
+ 'utils.gyp:utils',
],
},
{
@@ -192,16 +192,16 @@
'../src/utils/SkLua.cpp',
],
'dependencies': [
- 'skia_lib.gyp:skia_lib',
'effects.gyp:effects',
- 'utils.gyp:utils',
+ 'flags.gyp:flags',
'images.gyp:images',
+ 'lua.gyp:lua',
'tools.gyp:picture_renderer',
'tools.gyp:picture_utils',
'pdf.gyp:pdf',
'ports.gyp:ports',
- 'flags.gyp:flags',
- 'lua.gyp:lua',
+ 'skia_lib.gyp:skia_lib',
+ 'utils.gyp:utils',
],
},
{
@@ -217,10 +217,10 @@
'../src/pipe/utils/',
],
'dependencies': [
+ 'flags.gyp:flags',
'skia_lib.gyp:skia_lib',
'tools.gyp:picture_renderer',
'tools.gyp:picture_utils',
- 'flags.gyp:flags',
],
},
{
@@ -240,11 +240,11 @@
'../src/lazy/',
],
'dependencies': [
+ 'bench.gyp:bench_timer',
+ 'flags.gyp:flags',
'skia_lib.gyp:skia_lib',
'tools.gyp:picture_utils',
'tools.gyp:picture_renderer',
- 'bench.gyp:bench_timer',
- 'flags.gyp:flags',
],
},
{
@@ -268,10 +268,18 @@
'../src/pipe/utils/',
'../src/utils/',
],
+ 'direct_dependent_settings': {
+ 'include_dirs': [
+ # needed for JSON headers used within PictureRenderer.h
+ '../third_party/externals/jsoncpp-chromium/overrides/include/',
+ '../third_party/externals/jsoncpp/include/',
+ ],
+ },
'dependencies': [
+ 'flags.gyp:flags',
+ 'jsoncpp.gyp:jsoncpp',
'skia_lib.gyp:skia_lib',
'tools.gyp:picture_utils',
- 'flags.gyp:flags',
],
'conditions': [
['skia_gpu == 1',
@@ -296,8 +304,8 @@
'../src/utils/',
],
'dependencies': [
- 'skia_lib.gyp:skia_lib',
'pdf.gyp:pdf',
+ 'skia_lib.gyp:skia_lib',
'tools.gyp:picture_utils',
],
'conditions': [
@@ -359,9 +367,9 @@
'../tools/pinspect.cpp',
],
'dependencies': [
+ 'flags.gyp:flags',
'skia_lib.gyp:skia_lib',
'tools.gyp:picture_renderer',
- 'flags.gyp:flags',
],
},
{
@@ -379,11 +387,11 @@
'../bench/TimerData.cpp',
],
'dependencies': [
- 'skia_lib.gyp:skia_lib',
'bench.gyp:bench_timer',
- 'tools.gyp:picture_utils',
- 'tools.gyp:picture_renderer',
'flags.gyp:flags',
+ 'skia_lib.gyp:skia_lib',
+ 'tools.gyp:picture_renderer',
+ 'tools.gyp:picture_utils',
],
},
{
diff --git a/tools/PictureRenderer.cpp b/tools/PictureRenderer.cpp
index c53bd08..88ddc75 100644
--- a/tools/PictureRenderer.cpp
+++ b/tools/PictureRenderer.cpp
@@ -8,6 +8,7 @@
#include "PictureRenderer.h"
#include "picture_utils.h"
#include "SamplePipeControllers.h"
+#include "SkBitmapHasher.h"
#include "SkCanvas.h"
#include "SkData.h"
#include "SkDevice.h"
@@ -40,6 +41,39 @@
kDefaultTileHeight = 256
};
+/* 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_Failed[] = "failed";
+const static char kJsonKey_ActualResults_FailureIgnored[]= "failure-ignored";
+const static char kJsonKey_ActualResults_NoComparison[] = "no-comparison";
+const static char kJsonKey_ActualResults_Succeeded[] = "succeeded";
+const static char kJsonKey_ExpectedResults[] = "expected-results";
+const static char kJsonKey_ExpectedResults_AllowedDigests[] = "allowed-digests";
+const static char kJsonKey_ExpectedResults_IgnoreFailure[] = "ignore-failure";
+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));
+ Json::Value jsonTypeValuePair;
+ jsonTypeValuePair.append(Json::Value(kJsonKey_Hashtype_Bitmap_64bitMD5));
+ jsonTypeValuePair.append(Json::UInt64(hash));
+ fActualResultsNoComparison[testName] = jsonTypeValuePair;
+}
+
+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) {
SkASSERT(NULL == fPicture);
SkASSERT(NULL == fCanvas.get());
@@ -217,14 +251,28 @@
* @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 jsonSummaryPtr If not null, add image results to this summary.
* @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
*/
-static bool write(SkCanvas* canvas, SkString path) {
+static bool write(SkCanvas* canvas, const SkString* path, ImageResultsSummary *jsonSummaryPtr) {
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());
@@ -232,22 +280,33 @@
canvas->readPixels(&bitmap, 0, 0);
sk_tools::force_all_opaque(bitmap);
- // Since path is passed in by value, it is okay to modify it.
- path.append(".png");
- return SkImageEncoder::EncodeFile(path.c_str(), bitmap, SkImageEncoder::kPNG_Type, 100);
+ 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);
+ }
+
+ return SkImageEncoder::EncodeFile(fullPathname.c_str(), bitmap, SkImageEncoder::kPNG_Type, 100);
}
/**
- * If path is non NULL, append number to it, and call write(SkCanvas*, SkString) to write the
+ * 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) {
+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);
+ return write(canvas, &pathWithNumber, jsonSummaryPtr);
}
///////////////////////////////////////////////////////////////////////////////////////////////
@@ -309,7 +368,7 @@
writer.endRecording();
fCanvas->flush();
if (NULL != path) {
- return write(fCanvas, *path);
+ return write(fCanvas, path, fJsonSummaryPtr);
}
if (NULL != out) {
*out = SkNEW(SkBitmap);
@@ -340,7 +399,7 @@
fCanvas->drawPicture(*fPicture);
fCanvas->flush();
if (NULL != path) {
- return write(fCanvas, *path);
+ return write(fCanvas, path, fJsonSummaryPtr);
}
if (NULL != out) {
@@ -506,11 +565,22 @@
///////////////////////////////////////////////////////////////////////////////////////////////
-static void bitmapCopySubset(const SkBitmap& src, SkBitmap* dst, int xDst,
- int yDst) {
- for (int y = 0; y <src.height() && y + yDst < dst->height() ; y++) {
- for (int x = 0; x < src.width() && x + xDst < dst->width() ; x++) {
- *dst->getAddr32(xDst + x, yDst + y) = *src.getAddr32(x, y);
+/**
+ * Copies the entirety of the src bitmap (typically a tile) into a portion of the dst bitmap.
+ * If the src bitmap is too large to fit within the dst bitmap after the x and y
+ * offsets have been applied, any excess will be ignored (so only the top-left portion of the
+ * src bitmap will be copied).
+ *
+ * @param src source bitmap
+ * @param dst destination bitmap
+ * @param xOffset x-offset within destination bitmap
+ * @param yOffset y-offset within destination bitmap
+ */
+static void bitmapCopyAtOffset(const SkBitmap& src, SkBitmap* dst,
+ int xOffset, int yOffset) {
+ for (int y = 0; y <src.height() && y + yOffset < dst->height() ; y++) {
+ for (int x = 0; x < src.width() && x + xOffset < dst->width() ; x++) {
+ *dst->getAddr32(xOffset + x, yOffset + y) = *src.getAddr32(x, y);
}
}
}
@@ -545,12 +615,13 @@
for (int i = 0; i < fTileRects.count(); ++i) {
DrawTileToCanvas(fCanvas, fTileRects[i], fPicture);
if (NULL != path) {
- success &= writeAppendNumber(fCanvas, path, i);
+ success &= writeAppendNumber(fCanvas, path, i, fJsonSummaryPtr);
}
if (NULL != out) {
if (fCanvas->readPixels(&bitmap, 0, 0)) {
- bitmapCopySubset(bitmap, *out, SkScalarFloorToInt(fTileRects[i].left()),
- SkScalarFloorToInt(fTileRects[i].top()));
+ // Add this tile to the entire bitmap.
+ bitmapCopyAtOffset(bitmap, *out, SkScalarFloorToInt(fTileRects[i].left()),
+ SkScalarFloorToInt(fTileRects[i].top()));
} else {
success = false;
}
@@ -603,7 +674,7 @@
public:
CloneData(SkPicture* clone, SkCanvas* canvas, SkTDArray<SkRect>& rects, int start, int end,
- SkRunnable* done)
+ SkRunnable* done, ImageResultsSummary* jsonSummaryPtr)
: fClone(clone)
, fCanvas(canvas)
, fPath(NULL)
@@ -611,7 +682,8 @@
, fStart(start)
, fEnd(end)
, fSuccess(NULL)
- , fDone(done) {
+ , fDone(done)
+ , fJsonSummaryPtr(jsonSummaryPtr) {
SkASSERT(fDone != NULL);
}
@@ -626,7 +698,7 @@
for (int i = fStart; i < fEnd; i++) {
DrawTileToCanvas(fCanvas, fRects[i], fClone);
- if (fPath != NULL && !writeAppendNumber(fCanvas, fPath, i)
+ if ((fPath != NULL) && !writeAppendNumber(fCanvas, fPath, i, fJsonSummaryPtr)
&& fSuccess != NULL) {
*fSuccess = false;
// If one tile fails to write to a file, do not continue drawing the rest.
@@ -635,8 +707,8 @@
if (fBitmap != NULL) {
if (fCanvas->readPixels(&bitmap, 0, 0)) {
SkAutoLockPixels alp(*fBitmap);
- bitmapCopySubset(bitmap, fBitmap, SkScalarFloorToInt(fRects[i].left()),
- SkScalarFloorToInt(fRects[i].top()));
+ 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.
@@ -669,6 +741,7 @@
// and only set to false upon failure to write to a PNG.
SkRunnable* fDone;
SkBitmap* fBitmap;
+ ImageResultsSummary* fJsonSummaryPtr;
};
MultiCorePictureRenderer::MultiCorePictureRenderer(int threadCount)
@@ -704,7 +777,8 @@
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));
+ (pic, fCanvasPool[i], fTileRects, start, end, &fCountdown,
+ fJsonSummaryPtr));
}
}
diff --git a/tools/PictureRenderer.h b/tools/PictureRenderer.h
index 6a84130..6d886ab 100644
--- a/tools/PictureRenderer.h
+++ b/tools/PictureRenderer.h
@@ -11,6 +11,7 @@
#include "SkCanvas.h"
#include "SkCountdown.h"
#include "SkDrawFilter.h"
+#include "SkJSONCPP.h"
#include "SkMath.h"
#include "SkPaint.h"
#include "SkPicture.h"
@@ -37,6 +38,30 @@
class TiledPictureRenderer;
+/**
+ * Class for collecting image results (checksums) as we go.
+ */
+class ImageResultsSummary {
+public:
+ /**
+ * Adds this bitmap's hash to the summary of results.
+ *
+ * @param testName name of the test
+ * @param bitmap bitmap to store the hash of
+ */
+ void add(const char *testName, const SkBitmap& bitmap);
+
+ /**
+ * Writes the summary (as constructed so far) to a file.
+ *
+ * @param filename path to write the summary to
+ */
+ void writeToFile(const char *filename);
+
+private:
+ Json::Value fActualResultsNoComparison;
+};
+
class PictureRenderer : public SkRefCnt {
public:
@@ -183,6 +208,10 @@
fGridInfo.fTileInterval.set(width, height);
}
+ void setJsonSummaryPtr(ImageResultsSummary* jsonSummaryPtr) {
+ fJsonSummaryPtr = jsonSummaryPtr;
+ }
+
bool isUsingBitmapDevice() {
return kBitmap_DeviceType == fDeviceType;
}
@@ -266,6 +295,7 @@
PictureRenderer()
: fPicture(NULL)
+ , fJsonSummaryPtr(NULL)
, fDeviceType(kBitmap_DeviceType)
, fBBoxHierarchyType(kNone_BBoxHierarchyType)
, fScaleFactor(SK_Scalar1)
@@ -290,6 +320,7 @@
protected:
SkAutoTUnref<SkCanvas> fCanvas;
SkPicture* fPicture;
+ ImageResultsSummary* fJsonSummaryPtr;
SkDeviceTypes fDeviceType;
BBoxHierarchyType fBBoxHierarchyType;
DrawFilterFlags fDrawFilters[SkDrawFilter::kTypeCount];
diff --git a/tools/render_pictures_main.cpp b/tools/render_pictures_main.cpp
index 70bc221..b067a65 100644
--- a/tools/render_pictures_main.cpp
+++ b/tools/render_pictures_main.cpp
@@ -32,6 +32,8 @@
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.");
+DEFINE_string(writeJsonSummaryPath, "", "File to write a JSON summary of image results to. "
+ "TODO(epoger): Currently, this only works if --writePath is also specified.");
DEFINE_string2(writePath, w, "", "Directory to write the rendered images.");
DEFINE_bool(writeWholeImage, false, "In tile mode, write the entire rendered image to a "
"file, instead of an image for each tile.");
@@ -132,9 +134,12 @@
////////////////////////////////////////////////////////////////////////////////////////////////////
-static bool render_picture(const SkString& inputPath, const SkString* outputDir,
- sk_tools::PictureRenderer& renderer,
- SkBitmap** out) {
+/**
+ * Called only by render_picture().
+ */
+static bool render_picture_internal(const SkString& inputPath, const SkString* outputDir,
+ sk_tools::PictureRenderer& renderer,
+ SkBitmap** out) {
SkString inputFilename;
sk_tools::get_basename(&inputFilename, inputPath);
@@ -241,11 +246,21 @@
};
}
+/**
+ * Render the SKP file(s) within inputPath, writing their bitmap images into outputDir.
+ *
+ * @param inputPath path to an individual SKP file, or a directory of SKP files
+ * @param outputDir if not NULL, write the image(s) generated into this directory
+ * @param renderer PictureRenderer to use to render the SKPs
+ * @param jsonSummaryPtr if not NULL, add the image(s) generated to this summary
+ */
static bool render_picture(const SkString& inputPath, const SkString* outputDir,
- sk_tools::PictureRenderer& renderer) {
+ sk_tools::PictureRenderer& renderer,
+ sk_tools::ImageResultsSummary *jsonSummaryPtr) {
int diffs[256] = {0};
SkBitmap* bitmap = NULL;
- bool success = render_picture(inputPath,
+ renderer.setJsonSummaryPtr(jsonSummaryPtr);
+ bool success = render_picture_internal(inputPath,
FLAGS_writeWholeImage ? NULL : outputDir,
renderer,
FLAGS_validate || FLAGS_writeWholeImage ? &bitmap : NULL);
@@ -272,8 +287,8 @@
}
SkAutoTUnref<sk_tools::PictureRenderer> aurReferenceRenderer(referenceRenderer);
- success = render_picture(inputPath, NULL, *referenceRenderer,
- &referenceBitmap);
+ success = render_picture_internal(inputPath, NULL, *referenceRenderer,
+ &referenceBitmap);
if (!success || NULL == referenceBitmap || NULL == referenceBitmap->getPixels()) {
SkDebugf("Failed to draw the reference picture.\n");
@@ -327,7 +342,24 @@
if (FLAGS_writeWholeImage) {
sk_tools::force_all_opaque(*bitmap);
- if (NULL != outputDir && FLAGS_writeWholeImage) {
+
+ if (NULL != jsonSummaryPtr) {
+ // 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,
+ // we could combine results of different config types without conflicting filenames.
+ SkString hashFilename;
+ sk_tools::get_basename(&hashFilename, inputPath);
+ hashFilename.remove(hashFilename.size() - 4, 4); // Remove ".skp"
+ hashFilename.append(".png");
+ jsonSummaryPtr->add(hashFilename.c_str(), *bitmap);
+ }
+
+ if (NULL != outputDir) {
SkString inputFilename;
sk_tools::get_basename(&inputFilename, inputPath);
SkString outputPath;
@@ -347,7 +379,8 @@
static int process_input(const char* input, const SkString* outputDir,
- sk_tools::PictureRenderer& renderer) {
+ sk_tools::PictureRenderer& renderer,
+ sk_tools::ImageResultsSummary *jsonSummaryPtr) {
SkOSFile::Iter iter(input, "skp");
SkString inputFilename;
int failures = 0;
@@ -357,13 +390,13 @@
SkString inputPath;
SkString inputAsSkString(input);
sk_tools::make_filepath(&inputPath, inputAsSkString, inputFilename);
- if (!render_picture(inputPath, outputDir, renderer)) {
+ if (!render_picture(inputPath, outputDir, renderer, jsonSummaryPtr)) {
++failures;
}
} while(iter.next(&inputFilename));
} else if (SkStrEndsWith(input, ".skp")) {
SkString inputPath(input);
- if (!render_picture(inputPath, outputDir, renderer)) {
+ if (!render_picture(inputPath, outputDir, renderer, jsonSummaryPtr)) {
++failures;
}
} else {
@@ -427,10 +460,15 @@
if (FLAGS_writePath.count() == 1) {
outputDir.set(FLAGS_writePath[0]);
}
+ sk_tools::ImageResultsSummary jsonSummary;
+ sk_tools::ImageResultsSummary* jsonSummaryPtr = NULL;
+ if (FLAGS_writeJsonSummaryPath.count() == 1) {
+ jsonSummaryPtr = &jsonSummary;
+ }
int failures = 0;
for (int i = 0; i < FLAGS_readPath.count(); i ++) {
- failures += process_input(FLAGS_readPath[i], &outputDir, *renderer.get());
+ failures += process_input(FLAGS_readPath[i], &outputDir, *renderer.get(), jsonSummaryPtr);
}
if (failures != 0) {
SkDebugf("Failed to render %i pictures.\n", failures);
@@ -447,6 +485,9 @@
}
#endif
#endif
+ if (FLAGS_writeJsonSummaryPath.count() == 1) {
+ jsonSummary.writeToFile(FLAGS_writeJsonSummaryPath[0]);
+ }
return 0;
}