diff options
| -rw-r--r-- | tools/PictureRenderer.cpp | 52 | ||||
| -rw-r--r-- | tools/render_pictures_main.cpp | 8 | ||||
| -rwxr-xr-x | tools/tests/render_pictures_test.py | 257 |
3 files changed, 164 insertions, 153 deletions
diff --git a/tools/PictureRenderer.cpp b/tools/PictureRenderer.cpp index c712ae672f..e531a30dc0 100644 --- a/tools/PictureRenderer.cpp +++ b/tools/PictureRenderer.cpp @@ -323,17 +323,26 @@ uint32_t PictureRenderer::recordFlags() { } /** - * Write the canvas to an image file and/or JSON summary. + * Write the canvas to the specified path. * * @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; - * if empty, don't write out the image at all. + * @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 (checksum) to this summary. + * @param jsonSummaryPtr If not null, add image results 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. * - * @return bool True if the operation completed successfully. + * 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. */ static bool write(SkCanvas* canvas, const SkString& outputDir, const SkString& inputFilename, ImageResultsSummary *jsonSummaryPtr, bool useChecksumBasedFilenames, @@ -398,10 +407,8 @@ static bool write(SkCanvas* canvas, const SkString& outputDir, const SkString& i hash, tileNumberPtr); } - if (outputDir.isEmpty()) { - return true; - } - + SkASSERT(!outputDir.isEmpty()); // TODO(epoger): we want to remove this constraint, + // as noted above SkString dirPath; if (outputSubdirPtr) { dirPath = SkOSPath::SkPathJoin(outputDir.c_str(), outputSubdirPtr); @@ -463,13 +470,16 @@ bool PipePictureRenderer::render(SkBitmap** out) { 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 write(fCanvas, fOutputDir, fInputFilename, fJsonSummaryPtr, - fUseChecksumBasedFilenames); + return true; } SkString PipePictureRenderer::getConfigNameInternal() { @@ -493,13 +503,18 @@ bool SimplePictureRenderer::render(SkBitmap** out) { 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 write(fCanvas, fOutputDir, fInputFilename, fJsonSummaryPtr, - fUseChecksumBasedFilenames); + + return true; } SkString SimplePictureRenderer::getConfigNameInternal() { @@ -707,8 +722,10 @@ bool TiledPictureRenderer::render(SkBitmap** out) { bool success = true; for (int i = 0; i < fTileRects.count(); ++i) { draw_tile_to_canvas(fCanvas, fTileRects[i], fPicture); - success &= write(fCanvas, fOutputDir, fInputFilename, fJsonSummaryPtr, - fUseChecksumBasedFilenames, &i); + if (!fOutputDir.isEmpty()) { + success &= write(fCanvas, fOutputDir, fInputFilename, fJsonSummaryPtr, + fUseChecksumBasedFilenames, &i); + } if (NULL != out) { if (fCanvas->readPixels(&bitmap, 0, 0)) { // Add this tile to the entire bitmap. @@ -790,8 +807,9 @@ public: for (int i = fStart; i < fEnd; i++) { draw_tile_to_canvas(fCanvas, fRects[i], fClone); - if (!write(fCanvas, fOutputDir, fInputFilename, fJsonSummaryPtr, - fUseChecksumBasedFilenames, &i) + 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. diff --git a/tools/render_pictures_main.cpp b/tools/render_pictures_main.cpp index f11eb0b80e..352dbf38be 100644 --- a/tools/render_pictures_main.cpp +++ b/tools/render_pictures_main.cpp @@ -35,8 +35,10 @@ DEFINE_bool(writeChecksumBasedFilenames, false, 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."); -DEFINE_string2(writePath, w, "", "Directory to write the rendered images into."); +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_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 " @@ -339,6 +341,8 @@ static bool render_picture(const SkString& inputPath, const SkString* outputDir, 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 d378a54688..3ebed93578 100755 --- a/tools/tests/render_pictures_test.py +++ b/tools/tests/render_pictures_test.py @@ -26,92 +26,6 @@ EXPECTED_HEADER_CONTENTS = { "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): @@ -125,20 +39,13 @@ class RenderPicturesTest(base_unittest.TestCase): shutil.rmtree(self._temp_dir) def test_tiled_whole_image(self): - """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 - """ + """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_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', @@ -149,12 +56,22 @@ class RenderPicturesTest(base_unittest.TestCase): "header" : EXPECTED_HEADER_CONTENTS, "actual-results" : { "red.skp": { - "tiled-images": RED_TILES, - "whole-image": RED_WHOLEIMAGE, + # Manually verified: 640x400 red rectangle with black border + "whole-image": { + "checksumAlgorithm" : "bitmap-64bitMD5", + "checksumValue" : 11092453015575919668, + "comparisonResult" : "no-comparison", + "filepath" : "red_skp.png", + }, }, "green.skp": { - "tiled-images": GREEN_TILES, - "whole-image": GREEN_WHOLEIMAGE, + # Manually verified: 640x400 green rectangle with black border + "whole-image": { + "checksumAlgorithm" : "bitmap-64bitMD5", + "checksumValue" : 8891695120562235492, + "comparisonResult" : "no-comparison", + "filepath" : "green_skp.png", + }, } } } @@ -169,14 +86,28 @@ class RenderPicturesTest(base_unittest.TestCase): 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": { - "whole-image": RED_WHOLEIMAGE, + # Manually verified: 640x400 red rectangle with black border + "whole-image": { + "checksumAlgorithm" : "bitmap-64bitMD5", + "checksumValue" : 11092453015575919668, + "comparisonResult" : "no-comparison", + "filepath" : "red_skp.png", + }, }, "green.skp": { - "whole-image": GREEN_WHOLEIMAGE, + # Manually verified: 640x400 green rectangle with black border + "whole-image": { + "checksumAlgorithm" : "bitmap-64bitMD5", + "checksumValue" : 8891695120562235492, + "comparisonResult" : "no-comparison", + "filepath" : "green_skp.png", + }, } } } @@ -226,44 +157,36 @@ class RenderPicturesTest(base_unittest.TestCase): ['bitmap-64bitMD5_8891695120562235492.png']) def test_untiled_validate(self): - """Same as test_untiled, but with --validate.""" + """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') + """ output_json_path = os.path.join(self._temp_dir, 'output.json') self._generate_skps() - 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']) + with self.assertRaises(Exception): + self._run_render_pictures(['-r', self._input_skp_dir, + '--validate', + '--writePath', self._temp_dir, + '--writeJsonSummaryPath', output_json_path]) def test_untiled_without_writePath(self): - """Same as test_untiled, but without --writePath.""" + """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') + """ 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" : { - "red.skp": { - "whole-image": RED_WHOLEIMAGE, - }, - "green.skp": { - "whole-image": GREEN_WHOLEIMAGE, - } - } + "actual-results" : None, } self._assert_json_contents(output_json_path, expected_summary_dict) @@ -280,10 +203,76 @@ class RenderPicturesTest(base_unittest.TestCase): "header" : EXPECTED_HEADER_CONTENTS, "actual-results" : { "red.skp": { - "tiled-images": RED_TILES, + # 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", + }], }, "green.skp": { - "tiled-images": GREEN_TILES, + # 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", + }], } } } |