Merge lp:~linaro-graphics-wg/glmark2/options-finite-value-set into lp:glmark2/2011.11

[Alexandros Frantzis]

Add support for options that have a finite set of acceptable values.

This includes listing the values in --list-scenes, and warning the user if they use unacceptable values.

[Jesse Barker (jesse-barker)]

Looks good. I take it this makes it easier for us to present the options in the Android UI (i.e., rather than the user having to type option values, they might be able to select them from a list/menu of some sort)?

review: Approve

[Alexandros Frantzis (afrantzis)]

Yes. In fact, I already have some experimental code for Android GUI integration. It needs some cleanup, but if all goes well we can merge it in time for 2012.07.

=== modified file 'src/benchmark.cpp'
--- src/benchmark.cpp	2012-02-15 16:22:18 +0000
+++ src/benchmark.cpp	2012-07-17 14:39:31 +0000
@@ -144,7 +144,18 @@
          iter != options_.end();
          iter++)
     {
-        scene_.set_option(iter->first, iter->second);
+        if (!scene_.set_option(iter->first, iter->second)) {
+            map<string, Scene::Option>::const_iterator opt_iter = scene_.options().find(iter->first);
+
+            if (opt_iter == scene_.options().end()) {
+                Log::info("Warning: Scene '%s' doesn't accept option '%s'\n",
+                          scene_.name().c_str(), iter->first.c_str());
+            }
+            else {
+                Log::info("Warning: Scene '%s' doesn't accept value '%s' for option '%s'\n",
+                          scene_.name().c_str(), iter->second.c_str(), iter->first.c_str());
+            }
+        }
     }
 }
 
 
=== modified file 'src/main.cpp'
--- src/main.cpp	2012-07-09 15:54:30 +0000
+++ src/main.cpp	2012-07-17 14:39:31 +0000
@@ -99,6 +99,22 @@
                       opt.name.c_str(),
                       opt.description.c_str(),
                       opt.default_value.c_str());
+
+            /* Display list of acceptable values (if defined) */
+            if (!opt.acceptable_values.empty()) {
+                Log::info("    Acceptable Values: ");
+                for (vector<string>::const_iterator val_iter = opt.acceptable_values.begin();
+                     val_iter != opt.acceptable_values.end();
+                     val_iter++)
+                {
+                    std::string format_value(Log::continuation_prefix + "%s");
+                    if (val_iter + 1 != opt.acceptable_values.end())
+                        format_value += ",";
+                    else
+                        format_value += "\n";
+                    Log::info(format_value.c_str(), val_iter->c_str());
+                }
+            }
         }
     }
 }
 
=== modified file 'src/scene-buffer.cpp'
--- src/scene-buffer.cpp	2012-01-13 13:50:31 +0000
+++ src/scene-buffer.cpp	2012-07-17 14:39:31 +0000
@@ -303,9 +303,11 @@
 {
     priv_ = new SceneBufferPrivate();
     options_["interleave"] = Scene::Option("interleave", "false",
-                                           "Whether to interleave vertex attribute data [true,false]");
+                                           "Whether to interleave vertex attribute data",
+                                           "false,true");
     options_["update-method"] = Scene::Option("update-method", "map",
-                                              "[map,subdata]");
+                                              "Which method to use to update vertex data",
+                                              "map,subdata");
     options_["update-fraction"] = Scene::Option("update-fraction", "1.0",
                                                 "The fraction of the mesh length that is updated at every iteration (0.0-1.0)");
     options_["update-dispersion"] = Scene::Option("update-dispersion", "0.0",
@@ -315,7 +317,8 @@
     options_["rows"] = Scene::Option("rows", "20",
                                       "The number of mesh subdisivisions width-wise");
     options_["buffer-usage"] = Scene::Option("buffer-usage", "static",
-                                      "How the buffer will be used [static,stream,dynamic]");
+                                             "How the buffer will be used",
+                                             "static,stream,dynamic");
 }
 
 SceneBuffer::~SceneBuffer()
 
=== modified file 'src/scene-build.cpp'
--- src/scene-build.cpp	2012-05-22 08:46:33 +0000
+++ src/scene-build.cpp	2012-07-17 14:39:31 +0000
@@ -36,7 +36,7 @@
     orientModel_(false)
 {
     const ModelMap& modelMap = Model::find_models();
-    std::string optionDesc("Which model to use [");
+    std::string optionValues;
     for (ModelMap::const_iterator modelIt = modelMap.begin();
          modelIt != modelMap.end();
          modelIt++)
@@ -44,19 +44,20 @@
         static bool doSeparator(false);
         if (doSeparator)
         {
-            optionDesc += ", ";
+            optionValues += ",";
         }
         const std::string& curName = modelIt->first;
-        optionDesc += curName;
+        optionValues += curName;
         doSeparator = true;
     }
-    optionDesc += "]";
     options_["use-vbo"] = Scene::Option("use-vbo", "true",
-                                        "Whether to use VBOs for rendering [true,false]");
+                                        "Whether to use VBOs for rendering",
+                                        "false,true");
     options_["interleave"] = Scene::Option("interleave", "false",
-                                           "Whether to interleave vertex attribute data [true,false]");
-    options_["model"] = Scene::Option("model", "horse",
-                                      optionDesc);
+                                           "Whether to interleave vertex attribute data",
+                                           "false,true");
+    options_["model"] = Scene::Option("model", "horse", "Which model to use",
+                                      optionValues);
 }
 
 SceneBuild::~SceneBuild()
 
=== modified file 'src/scene-bump.cpp'
--- src/scene-bump.cpp	2012-05-15 18:38:47 +0000
+++ src/scene-bump.cpp	2012-07-17 14:39:31 +0000
@@ -35,7 +35,8 @@
     texture_(0), rotation_(0.0f), rotationSpeed_(0.0f)
 {
     options_["bump-render"] = Scene::Option("bump-render", "off",
-                                            "How to render bumps [off, normals, normals-tangent, height, high-poly]");
+                                            "How to render bumps",
+                                            "off,normals,normals-tangent,height,high-poly");
 }
 
 SceneBump::~SceneBump()
 
=== modified file 'src/scene-conditionals.cpp'
--- src/scene-conditionals.cpp	2011-11-11 11:07:15 +0000
+++ src/scene-conditionals.cpp	2012-07-17 14:39:31 +0000
@@ -42,11 +42,11 @@
     options_["fragment-steps"] = Scene::Option("fragment-steps", "1",
             "The number of computational steps in the fragment shader");
     options_["fragment-conditionals"] = Scene::Option("fragment-conditionals", "true",
-            "Whether each computational step includes an if-else clause");
+            "Whether each computational step includes an if-else clause", "false,true");
     options_["vertex-steps"] = Scene::Option("vertex-steps", "1",
             "The number of computational steps in the vertex shader");
     options_["vertex-conditionals"] = Scene::Option("vertex-conditionals", "true",
-            "Whether each computational step includes an if-else clause");
+            "Whether each computational step includes an if-else clause", "false,true");
 }
 
 SceneConditionals::~SceneConditionals()
 
=== modified file 'src/scene-default-options.cpp'
--- src/scene-default-options.cpp	2011-10-26 14:09:17 +0000
+++ src/scene-default-options.cpp	2012-07-17 14:39:31 +0000
@@ -21,6 +21,7 @@
  */
 #include "scene.h"
 #include "benchmark.h"
+#include "log.h"
 
 void
 SceneDefaultOptions::setup()
@@ -35,7 +36,18 @@
              scene_iter != scenes.end();
              scene_iter++)
         {
-            scene_iter->second->set_option_default(iter->first, iter->second);
+            Scene &scene(*(scene_iter->second));
+
+            /* 
+             * Display warning only if the option value is unsupported, not if
+             * the scene doesn't support the option at all.
+             */
+            if (!scene.set_option_default(iter->first, iter->second) &&
+                scene.options().find(iter->first) != scene.options().end())
+            {
+                Log::info("Warning: Scene '%s' doesn't accept default value '%s' for option '%s'\n",
+                          scene.name().c_str(), iter->second.c_str(), iter->first.c_str());
+            }
         }
     }
 }
 
=== modified file 'src/scene-desktop.cpp'
--- src/scene-desktop.cpp	2012-05-15 18:38:47 +0000
+++ src/scene-desktop.cpp	2012-07-17 14:39:31 +0000
@@ -759,8 +759,8 @@
     Scene(canvas, "desktop")
 {
     priv_ = new SceneDesktopPrivate(canvas);
-    options_["effect"] = Scene::Option("effect", "blur",
-                                       "the effect to use [blur]");
+    options_["effect"] = Scene::Option("effect", "blur", "The effect to use",
+                                       "blur,shadow");
     options_["windows"] = Scene::Option("windows", "4",
                                         "the number of windows");
     options_["window-size"] = Scene::Option("window-size", "0.35",
@@ -770,7 +770,8 @@
     options_["blur-radius"] = Scene::Option("blur-radius", "5",
                                             "the blur effect radius (in pixels)");
     options_["separable"] = Scene::Option("separable", "true",
-                                          "use separable convolution for the blur effect");
+                                          "use separable convolution for the blur effect",
+                                          "false,true");
     options_["shadow-size"] = Scene::Option("shadow-size", "20",
                                             "the size of the shadow (in pixels)");
 }
 
=== modified file 'src/scene-effect-2d.cpp'
--- src/scene-effect-2d.cpp	2012-05-15 18:38:47 +0000
+++ src/scene-effect-2d.cpp	2012-07-17 14:39:31 +0000
@@ -41,7 +41,8 @@
         "0,0,0;0,1,0;0,0,0",
         "The convolution kernel matrix to use [format: \"a,b,c...;d,e,f...\"");;
     options_["normalize"] = Scene::Option("normalize", "true",
-        "Whether to normalize the supplied convolution kernel matrix [true,false]");
+        "Whether to normalize the supplied convolution kernel matrix",
+        "false,true");
 }
 
 SceneEffect2D::~SceneEffect2D()
 
=== modified file 'src/scene-function.cpp'
--- src/scene-function.cpp	2011-11-11 11:07:15 +0000
+++ src/scene-function.cpp	2012-07-17 14:39:31 +0000
@@ -43,15 +43,15 @@
     options_["fragment-steps"] = Scene::Option("fragment-steps", "1",
             "The number of computational steps in the fragment shader");
     options_["fragment-function"] = Scene::Option("fragment-function", "true",
-            "Whether each computational step includes a function call");
+            "Whether each computational step includes a function call", "false,true");
     options_["vertex-steps"] = Scene::Option("vertex-steps", "1",
             "The number of computational steps in the vertex shader");
     options_["vertex-function"] = Scene::Option("vertex-function", "true",
-            "Whether each computational step includes an if-else clause");
+            "Whether each computational step includes an if-else clause", "false,true");
     options_["vertex-complexity"] = Scene::Option("vertex-complexity", "low",
-            "The complexity of each computational step in the vertex shader");
+            "The complexity of each computational step in the vertex shader", "low,medium");
     options_["fragment-complexity"] = Scene::Option("fragment-complexity", "low",
-            "The complexity of each computational step in the fragment shader");
+            "The complexity of each computational step in the fragment shader", "low,medium");
 }
 
 SceneFunction::~SceneFunction()
 
=== modified file 'src/scene-loop.cpp'
--- src/scene-loop.cpp	2011-11-11 11:07:15 +0000
+++ src/scene-loop.cpp	2012-07-17 14:39:31 +0000
@@ -42,15 +42,17 @@
     options_["fragment-steps"] = Scene::Option("fragment-steps", "1",
             "The number of computational steps in the fragment shader");
     options_["fragment-loop"] = Scene::Option("fragment-function", "true",
-            "Whether to execute the steps in the vertex shader using a for loop");
+            "Whether to execute the steps in the vertex shader using a for loop", "false,true");
     options_["vertex-steps"] = Scene::Option("vertex-steps", "1",
             "The number of computational steps in the vertex shader");
     options_["vertex-loop"] = Scene::Option("vertex-function", "true",
-            "Whether to execute the steps in the vertex shader using a for loop");
+            "Whether to execute the steps in the vertex shader using a for loop", "false,true");
     options_["vertex-uniform"] = Scene::Option("vertex-uniform", "true",
-            "Whether to use a uniform in the vertex shader for the number of loop iterations to perform (i.e. vertex-steps)");
+            "Whether to use a uniform in the vertex shader for the number of loop iterations to perform (i.e. vertex-steps)",
+            "false,true");
     options_["fragment-uniform"] = Scene::Option("fragment-uniform", "true",
-            "Whether to use a uniform in the fragment shader for the number of loop iterations to perform (i.e. fragment-steps)");
+            "Whether to use a uniform in the fragment shader for the number of loop iterations to perform (i.e. fragment-steps)",
+            "false,true");
 }
 
 SceneLoop::~SceneLoop()
 
=== modified file 'src/scene-pulsar.cpp'
--- src/scene-pulsar.cpp	2012-05-15 18:38:47 +0000
+++ src/scene-pulsar.cpp	2012-07-17 14:39:31 +0000
@@ -46,9 +46,12 @@
     texture_(0)
 {
     options_["quads"] = Scene::Option("quads", "5", "Number of quads to render");
-    options_["texture"] = Scene::Option("texture", "false", "Enable texturing");
-    options_["light"] = Scene::Option("light", "false", "Enable lighting");
-    options_["random"] = Scene::Option("random", "false", "Enable random rotation speeds");
+    options_["texture"] = Scene::Option("texture", "false", "Enable texturing",
+                                        "false,true");
+    options_["light"] = Scene::Option("light", "false", "Enable lighting",
+                                      "false,true");
+    options_["random"] = Scene::Option("random", "false", "Enable random rotation speeds",
+                                       "false,true");
 }
 
 ScenePulsar::~ScenePulsar()
 
=== modified file 'src/scene-shading.cpp'
--- src/scene-shading.cpp	2012-05-22 08:46:33 +0000
+++ src/scene-shading.cpp	2012-07-17 14:39:31 +0000
@@ -43,7 +43,7 @@
     orientModel_(false)
 {
     const ModelMap& modelMap = Model::find_models();
-    std::string optionDesc("Which model to use [");
+    std::string optionValues;
     for (ModelMap::const_iterator modelIt = modelMap.begin();
          modelIt != modelMap.end();
          modelIt++)
@@ -51,19 +51,19 @@
         static bool doSeparator(false);
         if (doSeparator)
         {
-            optionDesc += ", ";
+            optionValues += ",";
         }
         const std::string& curName = modelIt->first;
-        optionDesc += curName;
+        optionValues += curName;
         doSeparator = true;
     }
-    optionDesc += "]";
     options_["shading"] = Scene::Option("shading", "gouraud",
-                                        "[gouraud, blinn-phong-inf, phong]");
+                                        "Which shading method to use",
+                                        "gouraud,blinn-phong-inf,phong");
     options_["num-lights"] = Scene::Option("num-lights", "1",
             "The number of lights applied to the scene (phong only)");
-    options_["model"] = Scene::Option("model", "cat",
-                                      optionDesc);
+    options_["model"] = Scene::Option("model", "cat", "Which model to use",
+                                      optionValues);
 }
 
 SceneShading::~SceneShading()
 
=== modified file 'src/scene-terrain.cpp'
--- src/scene-terrain.cpp	2012-07-03 09:56:14 +0000
+++ src/scene-terrain.cpp	2012-07-17 14:39:31 +0000
@@ -218,9 +218,11 @@
     options_["repeat-overlay"] = Scene::Option("repeat-overlay", "6.0",
             "How many times to repeat the terrain texture on the terrain plane (per side)");
     options_["bloom"] = Scene::Option("bloom", "true",
-                                      "Use bloom post-processing effect [true,false]");
+                                      "Use bloom post-processing effect",
+                                      "false,true");
     options_["tilt-shift"] = Scene::Option("tilt-shift", "true",
-                                           "Use tilt-shift post-processing effect [true,false]");
+                                           "Use tilt-shift post-processing effect",
+                                           "false,true");
 }
 
 SceneTerrain::~SceneTerrain()
 
=== modified file 'src/scene-texture.cpp'
--- src/scene-texture.cpp	2012-07-04 10:07:42 +0000
+++ src/scene-texture.cpp	2012-07-17 14:39:31 +0000
@@ -42,7 +42,7 @@
     orientModel_(false), orientationAngle_(0.0)
 {
     const ModelMap& modelMap = Model::find_models();
-    string optionDesc("Which model to use [");
+    string optionValues;
     for (ModelMap::const_iterator modelIt = modelMap.begin();
          modelIt != modelMap.end();
          modelIt++)
@@ -50,18 +50,18 @@
         static bool doSeparator(false);
         if (doSeparator)
         {
-            optionDesc += ", ";
+            optionValues += ",";
         }
         const std::string& curName = modelIt->first;
-        optionDesc += curName;
+        optionValues += curName;
         doSeparator = true;
     }
-    optionDesc += "]";
-    options_["model"] = Scene::Option("model", "cube",
-                                      optionDesc);
+    options_["model"] = Scene::Option("model", "cube", "Which model to use",
+                                      optionValues);
     options_["texture-filter"] = Scene::Option("texture-filter", "nearest",
-                                               "[nearest, linear, linear-shader, mipmap]");
-    optionDesc = "Which texture to use [";
+                                               "The texture filter to use",
+                                               "nearest,linear,linear-shader,mipmap");
+    optionValues = "";
     const TextureMap& textureMap = Texture::find_textures();
     for (TextureMap::const_iterator textureIt = textureMap.begin();
          textureIt != textureMap.end();
@@ -70,17 +70,17 @@
         static bool doSeparator(false);
         if (doSeparator)
         {
-            optionDesc += ", ";
+            optionValues += ",";
         }
         const std::string& curName = textureIt->first;
-        optionDesc += curName;
+        optionValues += curName;
         doSeparator = true;
     }
-    optionDesc += "]";
-    options_["texture"] = Scene::Option("texture", "crate-base",
-                                        optionDesc);
+    options_["texture"] = Scene::Option("texture", "crate-base", "Which texture to use",
+                                        optionValues);
     options_["texgen"] = Scene::Option("texgen", "false",
-                                       "Whether to generate texcoords in the shader");
+                                       "Whether to generate texcoords in the shader",
+                                       "false,true");
 }
 
 SceneTexture::~SceneTexture()
 
=== modified file 'src/scene.cpp'
--- src/scene.cpp	2012-02-15 16:22:18 +0000
+++ src/scene.cpp	2012-07-17 14:39:31 +0000
@@ -28,11 +28,19 @@
 #include "util.h"
 #include <sstream>
 #include <cmath>
+#include <algorithm>
 
 using std::stringstream;
 using std::string;
 using std::map;
 
+Scene::Option::Option(const std::string &nam, const std::string &val, const std::string &desc,
+                      const std::string &values) :
+name(nam), value(val), default_value(val), description(desc), set(false)
+{
+    Util::split(values, ',', acceptable_values);
+}
+
 Scene::Scene(Canvas &pCanvas, const string &name) :
     canvas_(pCanvas), name_(name),
     startTime_(0), lastUpdateTime_(0), currentFrame_(0),
@@ -48,7 +56,8 @@
                                                    "The precision values for the fragment shader (\"int,float,sampler2d,samplercube\")");
     /* FPS options */
     options_["show-fps"] = Scene::Option("show-fps", "false",
-                                         "Show live FPS counter");
+                                         "Show live FPS counter",
+                                         "false,true");
     options_["fps-pos"] = Scene::Option("fps-pos", "-1.0,-1.0",
                                          "The position on screen where to show FPS");
     options_["fps-size"] = Scene::Option("fps-size", "0.03",
@@ -143,6 +152,14 @@
     if (iter == options_.end())
         return false;
 
+    std::vector<std::string> &values(iter->second.acceptable_values);
+
+    if (!values.empty() && 
+        std::find(values.begin(), values.end(), val) == values.end())
+    {
+            return false;
+    }
+
     iter->second.value = val;
     iter->second.set = true;
 
@@ -171,6 +188,14 @@
     if (iter == options_.end())
         return false;
 
+    std::vector<std::string> &values(iter->second.acceptable_values);
+
+    if (!values.empty() && 
+        std::find(values.begin(), values.end(), val) == values.end())
+    {
+            return false;
+    }
+
     iter->second.default_value = val;
 
     return true;
 
=== modified file 'src/scene.h'
--- src/scene.h	2012-07-09 15:54:30 +0000
+++ src/scene.h	2012-07-17 14:39:31 +0000
@@ -37,6 +37,7 @@
 #include <string>
 #include <map>
 #include <list>
+#include <vector>
 #include "canvas.h"
 
 /**
@@ -51,13 +52,15 @@
      * Scene options.
      */
     struct Option {
-        Option(const std::string &nam, const std::string &val, const std::string &desc) :
-            name(nam), value(val), default_value(val), description(desc), set(false) {}
+        Option(const std::string &nam, const std::string &val, const std::string &desc,
+               const std::string &values = "");
+
         Option() {}
         std::string name;
         std::string value;
         std::string default_value;
         std::string description;
+        std::vector<std::string> acceptable_values;
         bool set;
     };