Treat bad values passed to --images as a fatal error
If an option is passed to --images that is either a non-existent path or
a folder with no images matching the supported types, assume this is
an error and exit, so they can supply a valid path instead.
Share code between DM and nanobench in SkCommonFlags.
nanobench now behaves more like DM - it will check a directory for
images that match the supported extensions.
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1611323004
Review URL: https://codereview.chromium.org/1611323004
diff --git a/bench/nanobench.cpp b/bench/nanobench.cpp
index 7b37562..31efefa 100644
--- a/bench/nanobench.cpp
+++ b/bench/nanobench.cpp
@@ -586,19 +586,8 @@
fUseMPDs.push_back() = false;
// Prepare the images for decoding
- for (int i = 0; i < FLAGS_images.count(); i++) {
- const char* flag = FLAGS_images[i];
- if (sk_isdir(flag)) {
- // If the value passed in is a directory, add all the images
- SkOSFile::Iter it(flag);
- SkString file;
- while (it.next(&file)) {
- fImages.push_back() = SkOSPath::Join(flag, file.c_str());
- }
- } else if (sk_exists(flag)) {
- // Also add the value if it is a single image
- fImages.push_back() = flag;
- }
+ if (!CollectImages(&fImages)) {
+ exit(1);
}
// Choose the candidate color types for image decoding
diff --git a/dm/DM.cpp b/dm/DM.cpp
index 538e2f6..13a3f6b 100644
--- a/dm/DM.cpp
+++ b/dm/DM.cpp
@@ -558,7 +558,7 @@
return false;
}
-static void gather_srcs() {
+static bool gather_srcs() {
for (const skiagm::GMRegistry* r = skiagm::GMRegistry::Head(); r; r = r->next()) {
push_src("gm", "", new GMSrc(r->factory()));
}
@@ -573,33 +573,25 @@
push_src("skp", "", new SKPSrc(path));
}
}
- static const char* const exts[] = {
- "bmp", "gif", "jpg", "jpeg", "png", "webp", "ktx", "astc", "wbmp", "ico",
- "BMP", "GIF", "JPG", "JPEG", "PNG", "WEBP", "KTX", "ASTC", "WBMP", "ICO",
-#ifdef SK_CODEC_DECODES_RAW
- "arw", "cr2", "dng", "nef", "nrw", "orf", "raf", "rw2", "pef", "srw",
- "ARW", "CR2", "DNG", "NEF", "NRW", "ORF", "RAF", "RW2", "PEF", "SRW",
-#endif
- };
- for (int i = 0; i < FLAGS_images.count(); i++) {
- const char* flag = FLAGS_images[i];
- if (sk_isdir(flag)) {
- for (size_t j = 0; j < SK_ARRAY_COUNT(exts); j++) {
- SkOSFile::Iter it(flag, exts[j]);
- for (SkString file; it.next(&file); ) {
- SkString path = SkOSPath::Join(flag, file.c_str());
- push_codec_srcs(path);
- if (brd_supported(exts[j])) {
- push_brd_srcs(path);
- }
- }
- }
- } else if (sk_exists(flag)) {
- // assume that FLAGS_images[i] is a valid image if it is a file.
- push_codec_srcs(flag);
- push_brd_srcs(flag);
+
+ SkTArray<SkString> images;
+ if (!CollectImages(&images)) {
+ return false;
+ }
+
+ for (auto image : images) {
+ push_codec_srcs(image);
+ const char* ext = "";
+ int index = image.findLastOf('.');
+ if (index >= 0 && (size_t) ++index < image.size()) {
+ ext = &image.c_str()[index];
+ }
+ if (brd_supported(ext)) {
+ push_brd_srcs(image);
}
}
+
+ return true;
}
static void push_sink(const SkCommandLineConfig& config, Sink* s) {
@@ -1148,7 +1140,9 @@
gather_gold();
gather_uninteresting_hashes();
- gather_srcs();
+ if (!gather_srcs()) {
+ return 1;
+ }
gather_sinks();
gather_tests();
diff --git a/tools/flags/SkCommonFlags.cpp b/tools/flags/SkCommonFlags.cpp
index 2d5a855..e6d269b 100644
--- a/tools/flags/SkCommonFlags.cpp
+++ b/tools/flags/SkCommonFlags.cpp
@@ -6,6 +6,7 @@
*/
#include "SkCommonFlags.h"
+#include "SkOSFile.h"
DEFINE_bool(cpu, true, "master switch for running CPU-bound work.");
@@ -14,7 +15,8 @@
DEFINE_bool(gpu, true, "master switch for running GPU-bound work.");
-DEFINE_string(images, "", "Directory of images to decode.");
+DEFINE_string(images, "", "List of images and/or directories to decode. A directory with no images"
+ " is treated as a fatal error.");
DEFINE_string2(match, m, nullptr,
"[~][^]substring[$] [...] of GM name to run.\n"
@@ -47,5 +49,46 @@
"Space-separated key/value pairs to add to JSON identifying this builder.");
DEFINE_string(properties, "",
"Space-separated key/value pairs to add to JSON identifying this run.");
-
DEFINE_bool2(pre_log, p, false, "Log before running each test. May be incomprehensible when threading");
+
+bool CollectImages(SkTArray<SkString>* output) {
+ SkASSERT(output);
+
+ static const char* const exts[] = {
+ "bmp", "gif", "jpg", "jpeg", "png", "webp", "ktx", "astc", "wbmp", "ico",
+ "BMP", "GIF", "JPG", "JPEG", "PNG", "WEBP", "KTX", "ASTC", "WBMP", "ICO",
+#ifdef SK_CODEC_DECODES_RAW
+ "arw", "cr2", "dng", "nef", "nrw", "orf", "raf", "rw2", "pef", "srw",
+ "ARW", "CR2", "DNG", "NEF", "NRW", "ORF", "RAF", "RW2", "PEF", "SRW",
+#endif
+ };
+
+ for (int i = 0; i < FLAGS_images.count(); ++i) {
+ const char* flag = FLAGS_images[i];
+ if (!sk_exists(flag)) {
+ SkDebugf("%s does not exist!\n", flag);
+ return false;
+ }
+
+ if (sk_isdir(flag)) {
+ // If the value passed in is a directory, add all the images
+ bool foundAnImage = false;
+ for (const char* ext : exts) {
+ SkOSFile::Iter it(flag, ext);
+ SkString file;
+ while (it.next(&file)) {
+ foundAnImage = true;
+ output->push_back() = SkOSPath::Join(flag, file.c_str());
+ }
+ }
+ if (!foundAnImage) {
+ SkDebugf("No supported images found in %s!\n", flag);
+ return false;
+ }
+ } else {
+ // Also add the value if it is a single image
+ output->push_back() = flag;
+ }
+ }
+ return true;
+}
diff --git a/tools/flags/SkCommonFlags.h b/tools/flags/SkCommonFlags.h
index 127d373..8e00e75 100644
--- a/tools/flags/SkCommonFlags.h
+++ b/tools/flags/SkCommonFlags.h
@@ -9,6 +9,8 @@
#define SK_COMMON_FLAGS_H
#include "SkCommandLineFlags.h"
+#include "SkTArray.h"
+#include "SkString.h"
DECLARE_bool(cpu);
DECLARE_bool(dryRun);
@@ -30,4 +32,18 @@
DECLARE_string(key);
DECLARE_string(properties);
+/**
+ * Helper to assist in collecting image paths from --images.
+ *
+ * Populates an array of strings with paths to images to test.
+ *
+ * Returns true if each argument to --images is meaningful:
+ * - If the file/directory does not exist, return false.
+ * - If a directory passed to --images does not have any supported images (based on file
+ * type), return false.
+ * - If a file is passed to --images, assume the user is deliberately testing this image,
+ * regardless of file type.
+ */
+bool CollectImages(SkTArray<SkString>*);
+
#endif