Add config options to run different GPU APIs to dm and nanobench

dm/DM.cpp

Index: dm/DM.cpp
diff --git a/dm/DM.cpp b/dm/DM.cpp
index 6549af5a0fdb3ddc606191d4c736238fbf1a7963..bbd63b40759b54cfbc7464941ebd94d8993e55ae 100644
--- a/dm/DM.cpp
+++ b/dm/DM.cpp
@@ -15,6 +15,7 @@
 #include "SkChecksum.h"
 #include "SkCodec.h"
 #include "SkCommonFlags.h"
+#include "SkCommonFlagsConfig.h"
 #include "SkFontMgr.h"
 #include "SkForceLinking.h"
 #include "SkGraphics.h"
@@ -195,7 +196,7 @@ struct TaggedSrc : public SkAutoTDelete<Src> {
 };
 
 struct TaggedSink : public SkAutoTDelete<Sink> {
-    const char* tag;
+    SkString tag;
 };
 
 static const bool kMemcpyOK = true;
@@ -555,17 +556,9 @@ static void gather_srcs() {
     }
 }
 
-static GrGLStandard get_gpu_api() {
-    if (FLAGS_gpuAPI.contains("gl"))   { return kGL_GrGLStandard; }
-    if (FLAGS_gpuAPI.contains("gles")) { return kGLES_GrGLStandard; }
-    return kNone_GrGLStandard;
-}
-
-static void push_sink(const char* tag, Sink* s) {
+static void push_sink(const SkCommandLineConfig& config, Sink* s) {
     SkAutoTDelete<Sink> sink(s);
-    if (!FLAGS_config.contains(tag)) {
-        return;
-    }
+
     // Try a simple Src as a canary.  If it fails, skip this sink.
     struct : public Src {
         Error draw(SkCanvas* c) const override {
@@ -581,13 +574,13 @@ static void push_sink(const char* tag, Sink* s) {
     SkString log;
     Error err = sink->draw(justOneRect, &bitmap, &stream, &log);
     if (err.isFatal()) {
-        SkDebugf("Could not run %s: %s\n", tag, err.c_str());
+        SkDebugf("Could not run %s: %s\n", config.getTag().c_str(), err.c_str());
         exit(1);
     }
 
     TaggedSink& ts = gSinks.push_back();
     ts.reset(sink.detach());
-    ts.tag = tag;
+    ts.tag = config.getTag();
 }
 
 static bool gpu_supported() {
@@ -598,30 +591,57 @@ static bool gpu_supported() {
 #endif
 }
 
-static Sink* create_sink(const char* tag) {
-#define SINK(t, sink, ...) if (0 == strcmp(t, tag)) { return new sink(__VA_ARGS__); }
+static Sink* create_sink(const SkCommandLineConfig* config) {
+#if SK_SUPPORT_GPU
     if (gpu_supported()) {
-        typedef GrContextFactory Gr;
-        const GrGLStandard api = get_gpu_api();
-        SINK("gpunull",       GPUSink, Gr::kNull_GLContextType,          api,  0, false, FLAGS_gpu_threading);
-        SINK("gpudebug",      GPUSink, Gr::kDebug_GLContextType,         api,  0, false, FLAGS_gpu_threading);
-        SINK("gpu",           GPUSink, Gr::kNative_GLContextType,        api,  0, false, FLAGS_gpu_threading);
-        SINK("gpudft",        GPUSink, Gr::kNative_GLContextType,        api,  0,  true, FLAGS_gpu_threading);
-        SINK("msaa4",         GPUSink, Gr::kNative_GLContextType,        api,  4, false, FLAGS_gpu_threading);
-        SINK("msaa16",        GPUSink, Gr::kNative_GLContextType,        api, 16, false, FLAGS_gpu_threading);
-        SINK("nvprmsaa4",     GPUSink, Gr::kNative_GLContextType, Gr::kEnableNVPR_GLContextOptions, api,  4,  true, FLAGS_gpu_threading);
-        SINK("nvprmsaa16",    GPUSink, Gr::kNative_GLContextType, Gr::kEnableNVPR_GLContextOptions, api, 16,  true, FLAGS_gpu_threading);
-    #if SK_ANGLE
-        SINK("angle",         GPUSink, Gr::kANGLE_GLContextType,         api,  0, false, FLAGS_gpu_threading);
-        SINK("angle-gl",      GPUSink, Gr::kANGLE_GL_GLContextType,      api,  0, false, FLAGS_gpu_threading);
-    #endif
-    #if SK_COMMAND_BUFFER
-        SINK("commandbuffer", GPUSink, Gr::kCommandBuffer_GLContextType, api,  0, false, FLAGS_gpu_threading);
-    #endif
-    #if SK_MESA
-        SINK("mesa",          GPUSink, Gr::kMESA_GLContextType,          api,  0, false, FLAGS_gpu_threading);
-    #endif
+        if (const SkCommandLineConfigGpu* gpuConfig = config->asConfigGpu()) {
+            GrContextFactory::GLContextType contextType = GrContextFactory::kNative_GLContextType;
+            GrContextFactory::GLContextOptions contextOptions = GrContextFactory::kNone_GLContextOptions;
+            switch (gpuConfig->getAPI()) {
+                case SkCommandLineConfigGpu::kNative_API:
+                    contextType = GrContextFactory::kNative_GLContextType;
+                    break;
+                case SkCommandLineConfigGpu::kGL_API:
+                    contextType = GrContextFactory::kGL_GLContextType;
+                    break;
+                case SkCommandLineConfigGpu::kGLES_API:
+                    contextType = GrContextFactory::kGLES_GLContextType;
+                    break;
+#if SK_ANGLE
+                case SkCommandLineConfigGpu::kANGLE_API:
+                    contextType = GrContextFactory::kANGLE_GLContextType;
+                    break;
+                case SkCommandLineConfigGpu::kANGLE_GL_API:
+                    contextType = GrContextFactory::kANGLE_GL_GLContextType;

[bsalomon, 2015/12/02 23:22:42]

do we need this conversion? Could GrContextFactory work with command line enum?
Or vice-versa, use the grcontextfactory enum in place of the
SkCommandLineConfigGpu?

[Kimmo Kinnunen, 2015/12/04 14:26:18]

The idea is that these are different things. GrContextFactory::GLContextType
represents contexts that GrContextFactory understands.
SkCommandLineConfigGpu::API represents the possible values that the parsing is
implemented for. Most likely 100% of the time (hopefully?) these are exactly the
same. Still, they're a bit different concept.

Similarly to "enableNVPR". The SkCommandLineConfigGpu does not refer to
"ContextOptions", because "context option" is not a concept of the
(user-visible) command line tools options.

In practice, it would be confusing to refer from SkCommandLineConfigGpu to
GrContextFactory, because they're in different libraries. For example,
visualbench does not "use" GrContextFactory, where as it could use the config
parsing in the future. Naturally, this being an enum the reference would work.
It'd be just confusing.

I don't know if it makes a practical difference. For me, there's no upside,
since the switch has already been written. I don't know if this is the most
important place to start removing apparent redundancy. Of course, I'm not saying
that would be a good rationale for not removing.

Given the above, feel free to make a decision, I'll act accordingly.

[bsalomon, 2015/12/07 14:33:19]

GrContextFactory really only exists for our test tools and it should only
enumerate the context types that we want to test. We don't necessarily have to
leave the enum in GrContextFactory.The enum could live somewhere in tools/.
Actually GrContextFactory could live there too as it isn't really part of the
gpu target proper, but rather a building block for writing test tools.

+                    break;
+#endif
+#if SK_COMMAND_BUFFER
+                case SkCommandLineConfigGpu::kCommandBuffer_API:
+                    contextType = GrContextFactory::kCommandBuffer_GLContextType;
+                    break;
+#endif
+#if SK_MESA
+                case SkCommandLineConfigGpu::kMESA_API:
+                    contextType = GrContextFactory::kMESA_GLContextType;
+                    break;
+#endif
+                case SkCommandLineConfigGpu::kNull_API:
+                    contextType = GrContextFactory::kNull_GLContextType;
+                    break;
+                case SkCommandLineConfigGpu::kDebug_API:
+                    contextType = GrContextFactory::kDebug_GLContextType;
+                    break;
+            }
+            if (gpuConfig->getUseNVPR()) {
+                contextOptions = static_cast<GrContextFactory::GLContextOptions>(
+                    contextOptions | GrContextFactory::kEnableNVPR_GLContextOptions);
+            }
+            return new GPUSink(contextType, contextOptions, gpuConfig->getSamples(), gpuConfig->getUseDIText(), FLAGS_gpu_threading);
+        }
     }
+#endif
+
+#define SINK(t, sink, ...) if (config->getTag().equals(t)) { return new sink(__VA_ARGS__); }
 
 #ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
     SINK("hwui",           HWUISink);
@@ -641,8 +661,8 @@ static Sink* create_sink(const char* tag) {
     return nullptr;
 }
 
-static Sink* create_via(const char* tag, Sink* wrapped) {
-#define VIA(t, via, ...) if (0 == strcmp(t, tag)) { return new via(__VA_ARGS__); }
+static Sink* create_via(const SkString& tag, Sink* wrapped) {
+#define VIA(t, via, ...) if (tag.equals(t)) { return new via(__VA_ARGS__); }
     VIA("twice",     ViaTwice,             wrapped);
     VIA("pipe",      ViaPipe,              wrapped);
     VIA("serialize", ViaSerialization,     wrapped);
@@ -673,17 +693,22 @@ static Sink* create_via(const char* tag, Sink* wrapped) {
 }
 
 static void gather_sinks() {
-    for (int i = 0; i < FLAGS_config.count(); i++) {
-        const char* config = FLAGS_config[i];
-        SkTArray<SkString> parts;
-        SkStrSplit(config, "-", &parts);
-
-        Sink* sink = nullptr;
-        for (int i = parts.count(); i-- > 0;) {
-            const char* part = parts[i].c_str();
-            Sink* next = (sink == nullptr) ? create_sink(part) : create_via(part, sink);
+    SkCommandLineConfigArray configs;
+    ParseConfigs(FLAGS_config, &configs);
+    for (int i = 0; i < configs.count(); i++) {
+        const SkCommandLineConfig& config = *configs[i];
+        Sink* sink = create_sink(&config);
+        if (sink == nullptr) {
+            SkDebugf("Skipping config %s: Don't understand '%s'.\n", config.getTag().c_str(), config.getTag().c_str());
+            break;
+        }
+
+        const SkTArray<SkString>& parts = config.getViaParts();
+        for (int j = parts.count(); j-- > 0;) {
+            const SkString& part = parts[j];
+            Sink* next = create_via(part, sink);
             if (next == nullptr) {
-                SkDebugf("Skipping %s: Don't understand '%s'.\n", config, part);
+                SkDebugf("Skipping config %s: Don't understand '%s'.\n", config.getTag().c_str(), part.c_str());
                 delete sink;
                 sink = nullptr;
                 break;
@@ -805,7 +830,7 @@ struct Task {
         //   - this Src / Sink combination is on the blacklist;
         //   - it's a dry run.
         SkString note(task->src->veto(task->sink->flags()) ? " (veto)" : "");
-        SkString whyBlacklisted = is_blacklisted(task->sink.tag, task->src.tag.c_str(),
+        SkString whyBlacklisted = is_blacklisted(task->sink.tag.c_str(), task->src.tag.c_str(),
                                                  task->src.options.c_str(), name.c_str());
         if (!whyBlacklisted.isEmpty()) {
             note.appendf(" (--blacklist %s)", whyBlacklisted.c_str());
@@ -817,15 +842,15 @@ struct Task {
             SkBitmap bitmap;
             SkDynamicMemoryWStream stream;
             if (FLAGS_pre_log) {
-                SkDebugf("\nRunning %s->%s", name.c_str(), task->sink.tag);
+                SkDebugf("\nRunning %s->%s", name.c_str(), task->sink.tag.c_str());
             }
-            start(task->sink.tag, task->src.tag, task->src.options, name.c_str());
+            start(task->sink.tag.c_str(), task->src.tag, task->src.options, name.c_str());
             Error err = task->sink->draw(*task->src, &bitmap, &stream, &log);
             if (!err.isEmpty()) {
                 auto elapsed = now_ms() - timerStart;
                 if (err.isFatal()) {
                     fail(SkStringPrintf("%s %s %s %s: %s",
-                                        task->sink.tag,
+                                        task->sink.tag.c_str(),
                                         task->src.tag.c_str(),
                                         task->src.options.c_str(),
                                         name.c_str(),
@@ -833,7 +858,7 @@ struct Task {
                 } else {
                     note.appendf(" (skipped: %s)", err.c_str());
                 }
-                done(elapsed, task->sink.tag, task->src.tag, task->src.options,
+                done(elapsed, task->sink.tag.c_str(), task->src.tag, task->src.options,
                      name, note, log);
                 return;
             }
@@ -866,11 +891,11 @@ struct Task {
             }
 
             if (!FLAGS_readPath.isEmpty() &&
-                !gGold.contains(Gold(task->sink.tag, task->src.tag.c_str(),
+                !gGold.contains(Gold(task->sink.tag.c_str(), task->src.tag.c_str(),
                                      task->src.options.c_str(), name, md5))) {
                 fail(SkStringPrintf("%s not found for %s %s %s %s in %s",
                                     md5.c_str(),
-                                    task->sink.tag,
+                                    task->sink.tag.c_str(),
                                     task->src.tag.c_str(),
                                     task->src.options.c_str(),
                                     name.c_str(),
@@ -887,7 +912,7 @@ struct Task {
                 }
             }
         }
-        done(now_ms()-timerStart, task->sink.tag, task->src.tag.c_str(), task->src.options.c_str(),
+        done(now_ms()-timerStart, task->sink.tag.c_str(), task->src.tag.c_str(), task->src.options.c_str(),
              name, note, log);
     }
 
@@ -898,7 +923,7 @@ struct Task {
                             const SkBitmap* bitmap) {
         JsonWriter::BitmapResult result;
         result.name          = task.src->name();
-        result.config        = task.sink.tag;
+        result.config        = task.sink.tag.c_str();
         result.sourceType    = task.src.tag;
         result.sourceOptions = task.src.options;
         result.ext           = ext;
@@ -926,7 +951,7 @@ struct Task {
                 return;  // Content-addressed.  If it exists already, we're done.
             }
         } else {
-            path = SkOSPath::Join(dir, task.sink.tag);
+            path = SkOSPath::Join(dir, task.sink.tag.c_str());
             sk_mkdir(path.c_str());
             path = SkOSPath::Join(path.c_str(), task.src.tag.c_str());
             sk_mkdir(path.c_str());
@@ -1171,28 +1196,46 @@ template<typename T>
 void RunWithGPUTestContexts(T test, GPUTestContexts testContexts, Reporter* reporter,
                             GrContextFactory* factory) {
 #if SK_SUPPORT_GPU
-    const GrGLStandard api = get_gpu_api();
-    for (int i = 0; i < GrContextFactory::kGLContextTypeCnt; ++i) {
-        GrContextFactory::GLContextType glCtxType = (GrContextFactory::GLContextType) i;
+    // Iterate over context types, except use "native" instead of explicitly trying OpenGL and
+    // OpenGL ES. Do not use GLES on desktop, since tests do not account for not fixing
+    // http://skbug.com/2809
+    GrContextFactory::GLContextType contextTypes[] = {
+        GrContextFactory::kNative_GLContextType,
+#if SK_ANGLE
+        GrContextFactory::kANGLE_GLContextType,
+        GrContextFactory::kANGLE_GL_GLContextType,
+#endif
+#if SK_COMMAND_BUFFER
+        GrContextFactory::kCommandBuffer_GLContextType,
+#endif
+#if SK_MESA
+        GrContextFactory::kMESA_GLContextType,
+#endif
+        GrContextFactory::kNull_GLContextType,
+        GrContextFactory::kDebug_GLContextType,
+    };
+    static_assert(SK_ARRAY_COUNT(contextTypes) == GrContextFactory::kGLContextTypeCnt - 2,
+                  "Skipping unexpected GLContextType for GPU tests");
+
+    for (auto& contextType : contextTypes) {
         int contextSelector = kNone_GPUTestContexts;
-        if (GrContextFactory::IsRenderingGLContext(glCtxType)) {
+        if (GrContextFactory::IsRenderingGLContext(contextType)) {
             contextSelector |= kAllRendering_GPUTestContexts;
         }
-        if (glCtxType == GrContextFactory::kNative_GLContextType) {
+        if (contextType == GrContextFactory::kNative_GLContextType) {
             contextSelector |= kNative_GPUTestContexts;
         }
-        if (glCtxType == GrContextFactory::kNull_GLContextType) {
+        if (contextType == GrContextFactory::kNull_GLContextType) {
             contextSelector |= kNull_GPUTestContexts;
         }
         if ((testContexts & contextSelector) == 0) {
             continue;
         }
-        if (GrContextFactory::ContextInfo* context = factory->getContextInfo(glCtxType, api)) {
+        if (GrContextFactory::ContextInfo* context = factory->getContextInfo(contextType)) {
             call_test(test, reporter, context);
         }
         if (GrContextFactory::ContextInfo* context =
-            factory->getContextInfo(glCtxType, api,
-                                    GrContextFactory::kEnableNVPR_GLContextOptions)) {
+            factory->getContextInfo(contextType, GrContextFactory::kEnableNVPR_GLContextOptions)) {
             call_test(test, reporter, context);
         }
     }