Index: bench/nanobench.cpp |
diff --git a/bench/nanobench.cpp b/bench/nanobench.cpp |
index 7efd858c5cd5a8a0e761c399a839fa6964526e2b..070bcc6845a6fc4073842f2d4cb7655d2bf5e4c2 100644 |
--- a/bench/nanobench.cpp |
+++ b/bench/nanobench.cpp |
@@ -185,7 +185,7 @@ struct GPUTarget : public Target { |
} |
if (!this->gl->fenceSyncSupport()) { |
SkDebugf("WARNING: GL context for config \"%s\" does not support fence sync. " |
- "Timings might not be accurate.\n", this->config.name); |
+ "Timings might not be accurate.\n", this->config.name.c_str()); |
} |
return true; |
} |
@@ -411,14 +411,56 @@ static bool is_gpu_config_allowed(const char* name, GrContextFactory::GLContextT |
#define kBogusGLContextOptions 0 |
#endif |
+// create a config from a specific 'gpu(...)' parameter pack. |
Kimmo Kinnunen
2016/01/20 18:25:55
This comment does not add any information, if you
Sami Väisänen
2016/01/21 12:42:32
Done.
|
+static void create_commandline_config(SkTArray<Config, false>& nanoConfigs, const SkCommandLineConfig& conf) { |
Kimmo Kinnunen
2016/01/20 18:25:55
So this function could be called
static void creat
Sami Väisänen
2016/01/21 12:42:32
Done.
|
+ if (!FLAGS_gpu) |
+ return; |
Kimmo Kinnunen
2016/01/20 18:25:55
this would be removed.
Instead, you actually need
Sami Väisänen
2016/01/21 12:42:32
Done.
|
+ |
+ // only interested in gpu configs. |
+ const auto* gpuConf = conf.asConfigGpu(); |
Kimmo Kinnunen
2016/01/20 18:25:55
This would go to the parent function.
|
+ if (!gpuConf) |
+ return; |
+ |
+ // if tag is empty the config was not specified with parameters, |
+ // i.e. it was just simple config. |
+ const auto& tag = gpuConf->getTag(); |
+ if (tag.isEmpty()) |
Kimmo Kinnunen
2016/01/20 18:25:55
This is wrong. The intention is that you check con
|
+ return; |
+ |
Kimmo Kinnunen
2016/01/20 18:25:55
you would need to call also the is_gpu_config_allo
Sami Väisänen
2016/01/21 12:42:32
Done.
|
+ Config target; |
Kimmo Kinnunen
2016/01/20 18:25:55
So here you handle all GPU related configs. Thus y
Sami Väisänen
2016/01/21 12:42:32
Done.
|
+ target.name = tag; |
Kimmo Kinnunen
2016/01/20 18:25:56
Since the original used object initializer syntax,
Sami Väisänen
2016/01/21 12:42:32
Done.
|
+ target.backend = Benchmark::kGPU_Backend; |
+ target.color = kN32_SkColorType; |
+ target.alpha = kPremul_SkAlphaType; |
+ target.ctxOptions = gpuConf->getUseNVPR() ? GrContextFactory::kEnableNVPR_GLContextOptions : |
+ GrContextFactory::kNone_GLContextOptions; |
+ target.samples = gpuConf->getSamples(); |
+ target.ctxType = gpuConf->getContextType(); |
+ target.useDFText = false; |
+ nanoConfigs.push_back(target); |
+} |
+ |
+ |
+ |
+// create configs from specific command line parameters |
+// that were specified using --config 'gpu(foo=bar,...)' parameters |
Kimmo Kinnunen
2016/01/20 18:25:55
The comment doesn't add that much value. It's rath
Sami Väisänen
2016/01/21 12:42:32
Done.
|
+static void gather_commandline_configs(SkTArray<Config, false>& nanoConfigs) { |
Kimmo Kinnunen
2016/01/20 18:25:55
So this could be called
static void create_config
Sami Väisänen
2016/01/21 12:42:32
Done.
|
+ SkCommandLineConfigArray configs; |
+ ParseConfigs(FLAGS_config, &configs); |
+ for (int i=0; i<configs.count(); ++i) { |
Kimmo Kinnunen
2016/01/20 18:25:55
Here whitespace between operators.
Sami Väisänen
2016/01/21 12:42:32
Done.
|
+ const auto& config = *configs[i]; |
Kimmo Kinnunen
2016/01/20 18:25:55
so here you should have code like
#if SK_SUPPORT_G
Sami Väisänen
2016/01/21 12:42:32
Done.
|
+ create_commandline_config(nanoConfigs, config); |
+ } |
+} |
+ |
// Append all configs that are enabled and supported. |
-static void create_configs(SkTDArray<Config>* configs) { |
+static void create_configs(SkTArray<Config, false>* configs) { |
Kimmo Kinnunen
2016/01/20 18:25:55
This would be called:
static void create_config(co
Sami Väisänen
2016/01/21 12:42:32
Done.
|
#define CPU_CONFIG(name, backend, color, alpha) \ |
if (is_cpu_config_allowed(#name)) { \ |
Kimmo Kinnunen
2016/01/20 18:25:56
here you would have
if (is_cpu_config_allowed(#nam
|
- Config config = { #name, Benchmark::backend, color, alpha, 0, \ |
+ Config config = { SkString(#name), Benchmark::backend, color, alpha, 0, \ |
kBogusGLContextType, kBogusGLContextOptions, \ |
false }; \ |
- configs->push(config); \ |
+ configs->push_back(config); \ |
Kimmo Kinnunen
2016/01/20 18:25:55
in the macro, you would add "return", so that the
|
} |
if (FLAGS_cpu) { |
@@ -432,7 +474,7 @@ static void create_configs(SkTDArray<Config>* configs) { |
if (is_gpu_config_allowed(#name, GrContextFactory::ctxType, \ |
Kimmo Kinnunen
2016/01/20 18:25:55
(1) GPU configs are already handled in the functio
Sami Väisänen
2016/01/21 12:42:32
Done.
|
GrContextFactory::ctxOptions, samples)) { \ |
Config config = { \ |
- #name, \ |
+ SkString(#name), \ |
Benchmark::kGPU_Backend, \ |
kN32_SkColorType, \ |
kPremul_SkAlphaType, \ |
@@ -440,7 +482,7 @@ static void create_configs(SkTDArray<Config>* configs) { |
GrContextFactory::ctxType, \ |
GrContextFactory::ctxOptions, \ |
useDFText }; \ |
- configs->push(config); \ |
+ configs->push_back(config); \ |
Kimmo Kinnunen
2016/01/20 18:30:46
Also make sure that macro line continuations match
Sami Väisänen
2016/01/21 12:42:32
Done.
|
} |
if (FLAGS_gpu) { |
@@ -472,7 +514,7 @@ static void create_configs(SkTDArray<Config>* configs) { |
Config config = { "hwui", Benchmark::kHWUI_Backend, kRGBA_8888_SkColorType, |
kPremul_SkAlphaType, 0, kBogusGLContextType, kBogusGLContextOptions, |
false }; |
- configs->push(config); |
+ configs->push_back(config); |
} |
#endif |
Kimmo Kinnunen
2016/01/20 18:25:55
Not sure about this, but here you could propose to
Sami Väisänen
2016/01/21 12:42:32
Done.
|
} |
@@ -1090,8 +1132,9 @@ int nanobench_main() { |
FLAGS_samples, "samples"); |
} |
- SkTDArray<Config> configs; |
+ SkTArray<Config, false> configs; |
Kimmo Kinnunen
2016/01/20 18:25:55
TArray MEM_COPY defaults to false, so it's maybe c
Sami Väisänen
2016/01/21 12:42:32
Done.
|
create_configs(&configs); |
+ gather_commandline_configs(configs); |
Kimmo Kinnunen
2016/01/20 18:25:55
and since create_configs would be your toplevel it
Sami Väisänen
2016/01/21 12:42:32
Done.
|
if (FLAGS_keepAlive) { |
start_keepalive(); |
@@ -1105,7 +1148,7 @@ int nanobench_main() { |
continue; |
} |
- if (!configs.isEmpty()) { |
+ if (!configs.empty()) { |
log->bench(bench->getUniqueName(), bench->getSize().fX, bench->getSize().fY); |
bench->delayedSetup(); |
} |
@@ -1117,7 +1160,7 @@ int nanobench_main() { |
// During HWUI output this canvas may be nullptr. |
SkCanvas* canvas = target->getCanvas(); |
- const char* config = target->config.name; |
+ const char* config = target->config.name.c_str(); |
target->setup(); |
bench->perCanvasPreDraw(canvas); |