reland r14391 ('fix contents of render_pictures JSON summary')
BUG=skia:2043,skia:2044,skia:1942,skia:2466
R=caryclark@google.com
TBR=caryclark
Author: epoger@google.com
Review URL: https://codereview.chromium.org/262453002
git-svn-id: http://skia.googlecode.com/svn/trunk@14443 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/tools/PictureRenderer.cpp b/tools/PictureRenderer.cpp
index e531a30..c712ae6 100644
--- a/tools/PictureRenderer.cpp
+++ b/tools/PictureRenderer.cpp
@@ -323,26 +323,17 @@
}
/**
- * Write the canvas to the specified path.
+ * Write the canvas to an image file and/or JSON summary.
*
* @param canvas Must be non-null. Canvas to be written to a file.
- * @param outputDir If nonempty, write the binary image to a file within this directory.
+ * @param outputDir If nonempty, write the binary image to a file within this directory;
+ * if empty, don't write out the image at all.
* @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 jsonSummaryPtr If not null, add image results (checksum) to this summary.
* @param useChecksumBasedFilenames If true, use checksum-based filenames when writing to disk.
* @param tileNumberPtr If not null, which tile number this image contains.
- * @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.
+ * @return bool True if the operation completed successfully.
*/
static bool write(SkCanvas* canvas, const SkString& outputDir, const SkString& inputFilename,
ImageResultsSummary *jsonSummaryPtr, bool useChecksumBasedFilenames,
@@ -407,8 +398,10 @@
hash, tileNumberPtr);
}
- SkASSERT(!outputDir.isEmpty()); // TODO(epoger): we want to remove this constraint,
- // as noted above
+ if (outputDir.isEmpty()) {
+ return true;
+ }
+
SkString dirPath;
if (outputSubdirPtr) {
dirPath = SkOSPath::SkPathJoin(outputDir.c_str(), outputSubdirPtr);
@@ -470,16 +463,13 @@
pipeCanvas->drawPicture(*fPicture);
writer.endRecording();
fCanvas->flush();
- 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;
+ return write(fCanvas, fOutputDir, fInputFilename, fJsonSummaryPtr,
+ fUseChecksumBasedFilenames);
}
SkString PipePictureRenderer::getConfigNameInternal() {
@@ -503,18 +493,13 @@
fCanvas->drawPicture(*fPicture);
fCanvas->flush();
- 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;
+ return write(fCanvas, fOutputDir, fInputFilename, fJsonSummaryPtr,
+ fUseChecksumBasedFilenames);
}
SkString SimplePictureRenderer::getConfigNameInternal() {
@@ -722,10 +707,8 @@
bool success = true;
for (int i = 0; i < fTileRects.count(); ++i) {
draw_tile_to_canvas(fCanvas, fTileRects[i], fPicture);
- if (!fOutputDir.isEmpty()) {
- success &= write(fCanvas, fOutputDir, fInputFilename, fJsonSummaryPtr,
- fUseChecksumBasedFilenames, &i);
- }
+ success &= write(fCanvas, fOutputDir, fInputFilename, fJsonSummaryPtr,
+ fUseChecksumBasedFilenames, &i);
if (NULL != out) {
if (fCanvas->readPixels(&bitmap, 0, 0)) {
// Add this tile to the entire bitmap.
@@ -807,9 +790,8 @@
for (int i = fStart; i < fEnd; i++) {
draw_tile_to_canvas(fCanvas, fRects[i], fClone);
- if ((!fOutputDir.isEmpty())
- && !write(fCanvas, fOutputDir, fInputFilename, fJsonSummaryPtr,
- fUseChecksumBasedFilenames, &i)
+ if (!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.
diff --git a/tools/render_pictures_main.cpp b/tools/render_pictures_main.cpp
index 352dbf3..f11eb0b 100644
--- a/tools/render_pictures_main.cpp
+++ b/tools/render_pictures_main.cpp
@@ -35,10 +35,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. "
- "See https://code.google.com/p/skia/issues/detail?id=2043 .");
-DEFINE_string2(writePath, w, "", "Directory to write the rendered images.");
+DEFINE_string(writeJsonSummaryPath, "", "File to write a JSON summary of image results to.");
+DEFINE_string2(writePath, w, "", "Directory to write the rendered images into.");
DEFINE_bool(writeWholeImage, false, "In tile mode, write the entire rendered image to a "
"file, instead of an image for each tile.");
DEFINE_bool(validate, false, "Verify that the rendered image contains the same pixels as "
@@ -341,8 +339,6 @@
if (FLAGS_writeWholeImage) {
sk_tools::force_all_opaque(*bitmap);
- // TODO(epoger): 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.
SkString inputFilename, outputPath;
sk_tools::get_basename(&inputFilename, inputPath);
sk_tools::make_filepath(&outputPath, *outputDir, inputFilename);
diff --git a/tools/tests/render_pictures_test.py b/tools/tests/render_pictures_test.py
index 3ebed93..d378a54 100755
--- a/tools/tests/render_pictures_test.py
+++ b/tools/tests/render_pictures_test.py
@@ -26,6 +26,92 @@
"revision" : 1,
}
+# Manually verified: 640x400 red rectangle with black border
+RED_WHOLEIMAGE = {
+ "checksumAlgorithm" : "bitmap-64bitMD5",
+ "checksumValue" : 11092453015575919668,
+ "comparisonResult" : "no-comparison",
+ "filepath" : "red_skp.png",
+}
+
+# Manually verified: 640x400 green rectangle with black border
+GREEN_WHOLEIMAGE = {
+ "checksumAlgorithm" : "bitmap-64bitMD5",
+ "checksumValue" : 8891695120562235492,
+ "comparisonResult" : "no-comparison",
+ "filepath" : "green_skp.png",
+}
+
+# Manually verified these 6 images, all 256x256 tiles,
+# consistent with a tiled version of the 640x400 red rect
+# with black borders.
+RED_TILES = [{
+ "checksumAlgorithm" : "bitmap-64bitMD5",
+ "checksumValue" : 5815827069051002745,
+ "comparisonResult" : "no-comparison",
+ "filepath" : "red_skp-tile0.png",
+},{
+ "checksumAlgorithm" : "bitmap-64bitMD5",
+ "checksumValue" : 9323613075234140270,
+ "comparisonResult" : "no-comparison",
+ "filepath" : "red_skp-tile1.png",
+}, {
+ "checksumAlgorithm" : "bitmap-64bitMD5",
+ "checksumValue" : 16670399404877552232,
+ "comparisonResult" : "no-comparison",
+ "filepath" : "red_skp-tile2.png",
+}, {
+ "checksumAlgorithm" : "bitmap-64bitMD5",
+ "checksumValue" : 2507897274083364964,
+ "comparisonResult" : "no-comparison",
+ "filepath" : "red_skp-tile3.png",
+}, {
+ "checksumAlgorithm" : "bitmap-64bitMD5",
+ "checksumValue" : 7325267995523877959,
+ "comparisonResult" : "no-comparison",
+ "filepath" : "red_skp-tile4.png",
+}, {
+ "checksumAlgorithm" : "bitmap-64bitMD5",
+ "checksumValue" : 2181381724594493116,
+ "comparisonResult" : "no-comparison",
+ "filepath" : "red_skp-tile5.png",
+}]
+
+# Manually verified these 6 images, all 256x256 tiles,
+# consistent with a tiled version of the 640x400 green rect
+# with black borders.
+GREEN_TILES = [{
+ "checksumAlgorithm" : "bitmap-64bitMD5",
+ "checksumValue" : 12587324416545178013,
+ "comparisonResult" : "no-comparison",
+ "filepath" : "green_skp-tile0.png",
+}, {
+ "checksumAlgorithm" : "bitmap-64bitMD5",
+ "checksumValue" : 7624374914829746293,
+ "comparisonResult" : "no-comparison",
+ "filepath" : "green_skp-tile1.png",
+}, {
+ "checksumAlgorithm" : "bitmap-64bitMD5",
+ "checksumValue" : 5686489729535631913,
+ "comparisonResult" : "no-comparison",
+ "filepath" : "green_skp-tile2.png",
+}, {
+ "checksumAlgorithm" : "bitmap-64bitMD5",
+ "checksumValue" : 7980646035555096146,
+ "comparisonResult" : "no-comparison",
+ "filepath" : "green_skp-tile3.png",
+}, {
+ "checksumAlgorithm" : "bitmap-64bitMD5",
+ "checksumValue" : 17817086664365875131,
+ "comparisonResult" : "no-comparison",
+ "filepath" : "green_skp-tile4.png",
+}, {
+ "checksumAlgorithm" : "bitmap-64bitMD5",
+ "checksumValue" : 10673669813016809363,
+ "comparisonResult" : "no-comparison",
+ "filepath" : "green_skp-tile5.png",
+}]
+
class RenderPicturesTest(base_unittest.TestCase):
@@ -39,13 +125,20 @@
shutil.rmtree(self._temp_dir)
def test_tiled_whole_image(self):
- """Run render_pictures with tiles and --writeWholeImage flag."""
+ """Run render_pictures with tiles and --writeWholeImage flag.
+
+ TODO(epoger): This test generates undesired results! The JSON summary
+ includes both whole-image and tiled-images (as it should), but only
+ whole-images are written out to disk. See http://skbug.com/2463
+
+ TODO(epoger): I noticed that when this is run without --writePath being
+ specified, this test writes red_skp.png and green_skp.png to the current
+ directory. We should fix that... if --writePath is not specified, this
+ probably shouldn't write out red_skp.png and green_skp.png at all!
+ See http://skbug.com/2464
+ """
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_skp.png and green_skp.png to the current
- # directory. We should fix that... if --writePath is not specified, this
- # probably shouldn't write out red_skp.png and green_skp.png at all!
self._run_render_pictures(['-r', self._input_skp_dir,
'--bbh', 'grid', '256', '256',
'--mode', 'tile', '256', '256',
@@ -56,22 +149,12 @@
"header" : EXPECTED_HEADER_CONTENTS,
"actual-results" : {
"red.skp": {
- # Manually verified: 640x400 red rectangle with black border
- "whole-image": {
- "checksumAlgorithm" : "bitmap-64bitMD5",
- "checksumValue" : 11092453015575919668,
- "comparisonResult" : "no-comparison",
- "filepath" : "red_skp.png",
- },
+ "tiled-images": RED_TILES,
+ "whole-image": RED_WHOLEIMAGE,
},
"green.skp": {
- # Manually verified: 640x400 green rectangle with black border
- "whole-image": {
- "checksumAlgorithm" : "bitmap-64bitMD5",
- "checksumValue" : 8891695120562235492,
- "comparisonResult" : "no-comparison",
- "filepath" : "green_skp.png",
- },
+ "tiled-images": GREEN_TILES,
+ "whole-image": GREEN_WHOLEIMAGE,
}
}
}
@@ -86,28 +169,14 @@
self._run_render_pictures(['-r', self._input_skp_dir,
'--writePath', self._temp_dir,
'--writeJsonSummaryPath', output_json_path])
- # TODO(epoger): These expectations are the same as for above unittest.
- # Define the expectations once, and share them.
expected_summary_dict = {
"header" : EXPECTED_HEADER_CONTENTS,
"actual-results" : {
"red.skp": {
- # Manually verified: 640x400 red rectangle with black border
- "whole-image": {
- "checksumAlgorithm" : "bitmap-64bitMD5",
- "checksumValue" : 11092453015575919668,
- "comparisonResult" : "no-comparison",
- "filepath" : "red_skp.png",
- },
+ "whole-image": RED_WHOLEIMAGE,
},
"green.skp": {
- # Manually verified: 640x400 green rectangle with black border
- "whole-image": {
- "checksumAlgorithm" : "bitmap-64bitMD5",
- "checksumValue" : 8891695120562235492,
- "comparisonResult" : "no-comparison",
- "filepath" : "green_skp.png",
- },
+ "whole-image": GREEN_WHOLEIMAGE,
}
}
}
@@ -157,36 +226,44 @@
['bitmap-64bitMD5_8891695120562235492.png'])
def test_untiled_validate(self):
- """Same as test_untiled, but with --validate.
-
- TODO(epoger): This test generates undesired results! The call
- to render_pictures should succeed, and generate the same output as
- test_untiled.
- See https://code.google.com/p/skia/issues/detail?id=2044 ('render_pictures:
- --validate fails')
- """
+ """Same as test_untiled, but with --validate."""
output_json_path = os.path.join(self._temp_dir, 'output.json')
self._generate_skps()
- with self.assertRaises(Exception):
- self._run_render_pictures(['-r', self._input_skp_dir,
- '--validate',
- '--writePath', self._temp_dir,
- '--writeJsonSummaryPath', output_json_path])
+ self._run_render_pictures(['-r', self._input_skp_dir,
+ '--validate',
+ '--writePath', self._temp_dir,
+ '--writeJsonSummaryPath', output_json_path])
+ expected_summary_dict = {
+ "header" : EXPECTED_HEADER_CONTENTS,
+ "actual-results" : {
+ "red.skp": {
+ "whole-image": RED_WHOLEIMAGE,
+ },
+ "green.skp": {
+ "whole-image": GREEN_WHOLEIMAGE,
+ }
+ }
+ }
+ self._assert_json_contents(output_json_path, expected_summary_dict)
+ self._assert_directory_contents(
+ self._temp_dir, ['red_skp.png', 'green_skp.png', 'output.json'])
def test_untiled_without_writePath(self):
- """Same as test_untiled, but without --writePath.
-
- TODO(epoger): This test generates undesired results!
- See https://code.google.com/p/skia/issues/detail?id=2043 ('render_pictures:
- --writeJsonSummaryPath fails unless --writePath is specified')
- """
+ """Same as test_untiled, but without --writePath."""
output_json_path = os.path.join(self._temp_dir, 'output.json')
self._generate_skps()
self._run_render_pictures(['-r', self._input_skp_dir,
'--writeJsonSummaryPath', output_json_path])
expected_summary_dict = {
"header" : EXPECTED_HEADER_CONTENTS,
- "actual-results" : None,
+ "actual-results" : {
+ "red.skp": {
+ "whole-image": RED_WHOLEIMAGE,
+ },
+ "green.skp": {
+ "whole-image": GREEN_WHOLEIMAGE,
+ }
+ }
}
self._assert_json_contents(output_json_path, expected_summary_dict)
@@ -203,76 +280,10 @@
"header" : EXPECTED_HEADER_CONTENTS,
"actual-results" : {
"red.skp": {
- # Manually verified these 6 images, all 256x256 tiles,
- # consistent with a tiled version of the 640x400 red rect
- # with black borders.
- "tiled-images": [{
- "checksumAlgorithm" : "bitmap-64bitMD5",
- "checksumValue" : 5815827069051002745,
- "comparisonResult" : "no-comparison",
- "filepath" : "red_skp-tile0.png",
- }, {
- "checksumAlgorithm" : "bitmap-64bitMD5",
- "checksumValue" : 9323613075234140270,
- "comparisonResult" : "no-comparison",
- "filepath" : "red_skp-tile1.png",
- }, {
- "checksumAlgorithm" : "bitmap-64bitMD5",
- "checksumValue" : 16670399404877552232,
- "comparisonResult" : "no-comparison",
- "filepath" : "red_skp-tile2.png",
- }, {
- "checksumAlgorithm" : "bitmap-64bitMD5",
- "checksumValue" : 2507897274083364964,
- "comparisonResult" : "no-comparison",
- "filepath" : "red_skp-tile3.png",
- }, {
- "checksumAlgorithm" : "bitmap-64bitMD5",
- "checksumValue" : 7325267995523877959,
- "comparisonResult" : "no-comparison",
- "filepath" : "red_skp-tile4.png",
- }, {
- "checksumAlgorithm" : "bitmap-64bitMD5",
- "checksumValue" : 2181381724594493116,
- "comparisonResult" : "no-comparison",
- "filepath" : "red_skp-tile5.png",
- }],
+ "tiled-images": RED_TILES,
},
"green.skp": {
- # Manually verified these 6 images, all 256x256 tiles,
- # consistent with a tiled version of the 640x400 green rect
- # with black borders.
- "tiled-images": [{
- "checksumAlgorithm" : "bitmap-64bitMD5",
- "checksumValue" : 12587324416545178013,
- "comparisonResult" : "no-comparison",
- "filepath" : "green_skp-tile0.png",
- }, {
- "checksumAlgorithm" : "bitmap-64bitMD5",
- "checksumValue" : 7624374914829746293,
- "comparisonResult" : "no-comparison",
- "filepath" : "green_skp-tile1.png",
- }, {
- "checksumAlgorithm" : "bitmap-64bitMD5",
- "checksumValue" : 5686489729535631913,
- "comparisonResult" : "no-comparison",
- "filepath" : "green_skp-tile2.png",
- }, {
- "checksumAlgorithm" : "bitmap-64bitMD5",
- "checksumValue" : 7980646035555096146,
- "comparisonResult" : "no-comparison",
- "filepath" : "green_skp-tile3.png",
- }, {
- "checksumAlgorithm" : "bitmap-64bitMD5",
- "checksumValue" : 17817086664365875131,
- "comparisonResult" : "no-comparison",
- "filepath" : "green_skp-tile4.png",
- }, {
- "checksumAlgorithm" : "bitmap-64bitMD5",
- "checksumValue" : 10673669813016809363,
- "comparisonResult" : "no-comparison",
- "filepath" : "green_skp-tile5.png",
- }],
+ "tiled-images": GREEN_TILES,
}
}
}