Revert of Treat bad values passed to --images as a fatal error (patchset #17 id:320001 of https://codereview.chromium.org/1611323004/ )
Reason for revert:
Speculative to fix windows bots
Original issue's description:
> 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.
>
> Only consider image paths ending in RAW suffixes as images if
> SK_CODE_DECODES_RAW is defined. This prevents us from seeing failure
> to decode errors on platforms that cannot decode it.
>
> GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1611323004
>
> Committed: https://skia.googlesource.com/skia/+/7579786f3bd5a8fda84a1abc45b16213c3371f93
TBR=mtklein@google.com,borenet@google.com,msarett@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
# Not skipping CQ checks because original CL landed more than 1 days ago.
Review URL: https://codereview.chromium.org/1653543002
diff --git a/bench/nanobench.cpp b/bench/nanobench.cpp
index 492cb0b..c2bcb49 100644
--- a/bench/nanobench.cpp
+++ b/bench/nanobench.cpp
@@ -581,8 +581,19 @@
fUseMPDs.push_back() = false;
// Prepare the images for decoding
- if (!CollectImages(&fImages)) {
- exit(1);
+ 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;
+ }
}
// Choose the candidate color types for image decoding
diff --git a/dm/DM.cpp b/dm/DM.cpp
index 02c66a8..b135b00 100644
--- a/dm/DM.cpp
+++ b/dm/DM.cpp
@@ -519,7 +519,7 @@
return false;
}
-static bool gather_srcs() {
+static void gather_srcs() {
for (const skiagm::GMRegistry* r = skiagm::GMRegistry::Head(); r; r = r->next()) {
push_src("gm", "", new GMSrc(r->factory()));
}
@@ -534,25 +534,31 @@
push_src("skp", "", new SKPSrc(path));
}
}
-
- 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);
+ 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",
+ "arw", "cr2", "dng", "nef", "nrw", "orf", "raf", "rw2", "pef", "srw",
+ "ARW", "CR2", "DNG", "NEF", "NRW", "ORF", "RAF", "RW2", "PEF", "SRW",
+ };
+ 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);
}
}
-
- return true;
}
static void push_sink(const SkCommandLineConfig& config, Sink* s) {
@@ -1100,9 +1106,7 @@
gather_gold();
gather_uninteresting_hashes();
- if (!gather_srcs()) {
- return 1;
- }
+ gather_srcs();
gather_sinks();
gather_tests();
diff --git a/gyp/codec.gyp b/gyp/codec.gyp
index 2043b35..596270a 100644
--- a/gyp/codec.gyp
+++ b/gyp/codec.gyp
@@ -68,7 +68,8 @@
'TURBO_HAS_SKIP',
],
'conditions': [
- ['skia_codec_decodes_raw', {
+ # FIXME: fix the support for ChromeOS [DNG SDK issue with clock_gettime()].
+ ['skia_codec_decodes_raw and skia_os != "chromeos"', {
'dependencies': [
'raw_codec',
],
@@ -76,7 +77,7 @@
],
}, {
# RAW codec needs exceptions. Due to that, it is a separate target. Its usage can be
- # controlled by skia_codec_decodes_raw flag.
+ # controlled by SK_CODEC_DECODES_RAW flag.
'target_name': 'raw_codec',
'product_name': 'raw_codec',
'type': 'static_library',
@@ -110,7 +111,13 @@
'include_dirs': [
'../include/codec',
],
+ 'defines': [
+ 'SK_CODEC_DECODES_RAW',
+ ],
},
+ 'defines': [
+ 'SK_CODEC_DECODES_RAW',
+ ],
'conditions': [
['skia_arch_type == "x86" or skia_arch_type == "arm"', {
'defines': [
diff --git a/gyp/common_conditions.gypi b/gyp/common_conditions.gypi
index 11f98bd..c7b19de 100644
--- a/gyp/common_conditions.gypi
+++ b/gyp/common_conditions.gypi
@@ -12,11 +12,6 @@
'SK_FORCE_DISTANCE_FIELD_TEXT=<(skia_force_distance_field_text)',
],
'conditions' : [
- [ 'skia_codec_decodes_raw', {
- 'defines': [
- 'SK_CODEC_DECODES_RAW',
- ],
- }],
['skia_pic', {
'cflags': [
'-fPIC',
diff --git a/gyp/common_variables.gypi b/gyp/common_variables.gypi
index 17c5364..abbe729 100644
--- a/gyp/common_variables.gypi
+++ b/gyp/common_variables.gypi
@@ -38,6 +38,10 @@
'variables': { # level 1
'angle_path%': '../',
+ # RAW codec needs exceptions. Due to that, it is a separate target. Its usage can be controlled
+ # by this variable.
+ 'skia_codec_decodes_raw%': 1,
+
'variables': { # level 2
# Variables needed by conditions list within the level-2 variables dict.
@@ -58,14 +62,6 @@
}, {
'skia_arch_type%': 'x86',
}],
- # RAW codec needs exceptions. Due to that, it is a separate target. Its usage can be
- # controlled by skia_codec_decodes_raw.
- ['skia_os == "chromeos"', {
- # FIXME: fix the support for ChromeOS [DNG SDK issue with clock_gettime()].
- 'skia_codec_decodes_raw%' : 0,
- }, {
- 'skia_codec_decodes_raw%' : 1,
- }],
],
'arm_version%': 0,
'arm_neon%': 0,
@@ -76,7 +72,6 @@
# so that siblings of the level-2 'variables' dict can see them.
# (skia_os will depend on skia_android_framework.)
'skia_android_framework%': '<(skia_android_framework)',
- 'skia_codec_decodes_raw%': '<(skia_codec_decodes_raw)',
'skia_arch_type%': '<(skia_arch_type)',
'arm_version%': '<(arm_version)',
'arm_neon%': '<(arm_neon)',
@@ -219,7 +214,6 @@
'skia_gpu_extra_tests_path%': '<(skia_gpu_extra_tests_path)',
'skia_stroke_path_rendering%': '<(skia_stroke_path_rendering)',
'skia_android_framework%': '<(skia_android_framework)',
- 'skia_codec_decodes_raw%': '<(skia_codec_decodes_raw)',
'skia_use_android_framework_defines%': '<(skia_use_android_framework_defines)',
'skia_use_system_json%': '<(skia_use_system_json)',
'skia_android_path_rendering%': '<(skia_android_path_rendering)',
diff --git a/tools/flags/SkCommonFlags.cpp b/tools/flags/SkCommonFlags.cpp
index e6d269b..2d5a855 100644
--- a/tools/flags/SkCommonFlags.cpp
+++ b/tools/flags/SkCommonFlags.cpp
@@ -6,7 +6,6 @@
*/
#include "SkCommonFlags.h"
-#include "SkOSFile.h"
DEFINE_bool(cpu, true, "master switch for running CPU-bound work.");
@@ -15,8 +14,7 @@
DEFINE_bool(gpu, true, "master switch for running GPU-bound work.");
-DEFINE_string(images, "", "List of images and/or directories to decode. A directory with no images"
- " is treated as a fatal error.");
+DEFINE_string(images, "", "Directory of images to decode.");
DEFINE_string2(match, m, nullptr,
"[~][^]substring[$] [...] of GM name to run.\n"
@@ -49,46 +47,5 @@
"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 8e00e75..127d373 100644
--- a/tools/flags/SkCommonFlags.h
+++ b/tools/flags/SkCommonFlags.h
@@ -9,8 +9,6 @@
#define SK_COMMON_FLAGS_H
#include "SkCommandLineFlags.h"
-#include "SkTArray.h"
-#include "SkString.h"
DECLARE_bool(cpu);
DECLARE_bool(dryRun);
@@ -32,18 +30,4 @@
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