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

Review URL: https://codereview.chromium.org/1611323004
This commit is contained in:
scroggo 2016-01-28 08:41:10 -08:00 committed by Commit bot
parent d8ff5b336e
commit 7579786f3b
7 changed files with 101 additions and 53 deletions

View File

@ -582,19 +582,8 @@ public:
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

View File

@ -519,7 +519,7 @@ static bool brd_supported(const char* ext) {
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()));
}
@ -534,31 +534,25 @@ static void gather_srcs() {
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",
"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);
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) {
@ -1106,7 +1100,9 @@ int dm_main() {
gather_gold();
gather_uninteresting_hashes();
gather_srcs();
if (!gather_srcs()) {
return 1;
}
gather_sinks();
gather_tests();

View File

@ -68,8 +68,7 @@
'TURBO_HAS_SKIP',
],
'conditions': [
# FIXME: fix the support for ChromeOS [DNG SDK issue with clock_gettime()].
['skia_codec_decodes_raw and skia_os != "chromeos"', {
['skia_codec_decodes_raw', {
'dependencies': [
'raw_codec',
],
@ -77,7 +76,7 @@
],
}, {
# RAW codec needs exceptions. Due to that, it is a separate target. Its usage can be
# controlled by SK_CODEC_DECODES_RAW flag.
# controlled by skia_codec_decodes_raw flag.
'target_name': 'raw_codec',
'product_name': 'raw_codec',
'type': 'static_library',
@ -111,13 +110,7 @@
'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': [

View File

@ -12,6 +12,11 @@
'SK_FORCE_DISTANCE_FIELD_TEXT=<(skia_force_distance_field_text)',
],
'conditions' : [
[ 'skia_codec_decodes_raw', {
'defines': [
'SK_CODEC_DECODES_RAW',
],
}],
['skia_pic', {
'cflags': [
'-fPIC',

View File

@ -38,10 +38,6 @@
'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.
@ -62,6 +58,14 @@
}, {
'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,
@ -72,6 +76,7 @@
# 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)',
@ -214,6 +219,7 @@
'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)',

View File

@ -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(dryRun, false,
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 @@ DEFINE_string(key, "",
"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;
}

View File

@ -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_bool(pre_log);
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