[client] implemented renderer specific option API

Please note the vsync and mipmap options are now specific to OpenGL
To configure them use the following options:

  -o opengl:mipmap=1
  -o opengl:vsync=0

client/Makefile

@@ -15,6 +15,7 @@ BIN     ?= bin
 CFLAGS  += -DBUILD_VERSION='"$(shell git describe --always --long --dirty --abbrev=10 --tags)"'
 
 OBJS	= main.o \
+	  lg-renderer.o \
 	  spice/spice.o \
 	  ivshmem/ivshmem.o \
 	  renderers/opengl.o

client/lg-renderer.c

@@ -0,0 +1,50 @@
+/*
+Looking Glass - KVM FrameRelay (KVMFR) Client
+Copyright (C) 2017 Geoffrey McRae <geoff@hostfission.com>
+https://looking-glass.hostfission.com
+
+This program is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free Software
+Foundation; either version 2 of the License, or (at your option) any later
+version.
+
+This program is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+PARTICULAR PURPOSE. See the GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License along with
+this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+Place, Suite 330, Boston, MA 02111-1307 USA
+*/
+
+#include "lg-renderer.h"
+
+#include <string.h>
+
+bool LG_RendererValidatorBool(const char * value)
+{
+  if (!value)
+    return false;
+
+  return
+    (strcasecmp(value, "1"      ) == 0) ||
+    (strcasecmp(value, "0"      ) == 0) ||
+    (strcasecmp(value, "true"   ) == 0) ||
+    (strcasecmp(value, "false"  ) == 0) ||
+    (strcasecmp(value, "yes"    ) == 0) ||
+    (strcasecmp(value, "no"     ) == 0) ||
+    (strcasecmp(value, "on"     ) == 0) ||
+    (strcasecmp(value, "off"    ) == 0) ||
+    (strcasecmp(value, "enable" ) == 0) ||
+    (strcasecmp(value, "disable") == 0);
+}
+
+bool LG_RendererValueToBool(const char * value)
+{
+  return
+    (strcasecmp(value, "1"      ) == 0) ||
+    (strcasecmp(value, "true"   ) == 0) ||
+    (strcasecmp(value, "yes"    ) == 0) ||
+    (strcasecmp(value, "on"     ) == 0) ||
+    (strcasecmp(value, "enable" ) == 0);
+}

client/lg-renderer.h

@@ -26,6 +26,7 @@ Place, Suite 330, Boston, MA 02111-1307 USA
 
 #define IS_LG_RENDERER_VALID(x) \
   ((x)->get_name       && \
+   (x)->create         && \
    (x)->initialize     && \
    (x)->configure      && \
    (x)->deconfigure    && \
@@ -36,14 +37,33 @@ Place, Suite 330, Boston, MA 02111-1307 USA
    (x)->on_mouse_event && \
    (x)->render)
 
+#define LGR_OPTION_COUNT(x) (sizeof(x) / sizeof(LG_RendererOpt))
+
+typedef bool(* LG_RendererOptValidator)(const char * value);
+typedef void(* LG_RendererOptHandler  )(void * opaque, const char * value);
+
+typedef struct LG_RendererOpt
+{
+  const char              * name;
+  const char              * desc;
+  LG_RendererOptValidator   validator;
+  LG_RendererOptHandler     handler;
+}
+LG_RendererOpt;
+
+typedef struct LG_RendererOptValue
+{
+  const LG_RendererOpt   * opt;
+  const char             * value;
+} LG_RendererOptValue;
+
+typedef LG_RendererOpt * LG_RendererOptions;
+
 typedef struct LG_RendererParams
 {
-  int           argc;
+  TTF_Font * font;
-  const char ** argv;
+  bool       showFPS;
-  TTF_Font *    font;
+  bool       vsync;
-  bool          showFPS;
-  bool          resample;
-  bool          vsync;
 }
 LG_RendererParams;
 
@@ -75,7 +95,8 @@ typedef enum LG_RendererCursor
 LG_RendererCursor;
 
 typedef const char * (* LG_RendererGetName       )();
-typedef bool         (* LG_RendererInitialize    )(void ** opaque, const LG_RendererParams params, Uint32 * sdlFlags);
+typedef bool         (* LG_RendererCreate        )(void ** opaque, const LG_RendererParams params);
+typedef bool         (* LG_RendererInitialize    )(void * opaque, Uint32 * sdlFlags);
 typedef bool         (* LG_RendererConfigure     )(void * opaque, SDL_Window *window, const LG_RendererFormat format);
 typedef void         (* LG_RendererDeConfigure   )(void * opaque);
 typedef void         (* LG_RendererDeInitialize  )(void * opaque);
@@ -88,7 +109,10 @@ typedef bool         (* LG_RendererRender        )(void * opaque);
 
 typedef struct LG_Renderer
 {
+  LG_RendererCreate       create;
   LG_RendererGetName      get_name;
+  LG_RendererOptions      options;
+  unsigned int            option_count;
   LG_RendererInitialize   initialize;
   LG_RendererConfigure    configure;
   LG_RendererDeConfigure  deconfigure;
@@ -100,4 +124,8 @@ typedef struct LG_Renderer
   LG_RendererOnFrameEvent on_frame_event;
   LG_RendererRender       render;
 }
-LG_Renderer;
+LG_Renderer;
+
+// generic option helpers
+bool LG_RendererValidatorBool(const char * value);
+bool LG_RendererValueToBool  (const char * value);

client/lg-renderers.h

@@ -27,4 +27,6 @@ const LG_Renderer * LG_Renderers[] =
 {
   &LGR_OpenGL,
   NULL // end of array sentinal
-};
+};
+
+#define LG_RENDERER_COUNT ((sizeof(LG_Renderers) / sizeof(LG_Renderer *)) - 1)

client/main.c

@@ -63,9 +63,16 @@ struct AppState
   unsigned int         shmSize;
 };
 
+typedef struct RenderOpts
+{
+  unsigned int          size;
+  unsigned int          argc;
+  LG_RendererOptValue * argv;
+}
+RendererOpts;
+
 struct AppParams
 {
-  bool         vsync;
   bool         autoResize;
   bool         allowResize;
   bool         keepAspect;
@@ -75,7 +82,6 @@ struct AppParams
   int          x, y;
   unsigned int w, h;
   const char * ivshmemSocket;
-  bool         useMipmap;
   bool         showFPS;
   bool         useSpice;
   const char * spiceHost;
@@ -84,15 +90,12 @@ struct AppParams
   bool         hideMouse;
   bool         ignoreQuit;
 
-  unsigned int  rendererOptSize;
+  RendererOpts rendererOpts[LG_RENDERER_COUNT];
-  unsigned int  rendererOptCount;
-  const char ** rendererOpts;
 };
 
 struct AppState  state;
 struct AppParams params =
 {
-  .vsync            = true,
   .autoResize       = false,
   .allowResize      = true,
   .keepAspect       = true,
@@ -104,7 +107,6 @@ struct AppParams params =
   .w                = 1024,
   .h                = 768,
   .ivshmemSocket    = "/tmp/ivshmem_socket",
-  .useMipmap        = true,
   .showFPS          = false,
   .useSpice         = true,
   .spiceHost        = "127.0.0.1",
@@ -720,33 +722,41 @@ int run()
   }
 
   LG_RendererParams lgrParams;
-  lgrParams.argc     = params.rendererOptCount;
-  lgrParams.argv     = params.rendererOpts;
   lgrParams.font     = state.font;
-  lgrParams.resample = params.useMipmap;
   lgrParams.showFPS  = params.showFPS;
-  lgrParams.vsync    = params.vsync;
   Uint32 sdlFlags;
 
   // probe for a a suitable renderer
-  for(const LG_Renderer **r = &LG_Renderers[0]; *r; ++r)
+  for(unsigned int i = 0; i < LG_RENDERER_COUNT; ++i)
   {
-    if (!IS_LG_RENDERER_VALID(*r))
+    const LG_Renderer *r    = LG_Renderers[i];
+    RendererOpts      *opts = &params.rendererOpts[i];
+
+    if (!IS_LG_RENDERER_VALID(r))
     {
-      DEBUG_ERROR("FIXME: Renderer %d is invalid, skipping", (int)(r - &LG_Renderers[0]));
+      DEBUG_ERROR("FIXME: Renderer %d is invalid, skipping", i);
       continue;
     }
 
+    // create the renderer
     state.lgrData = NULL;
-    sdlFlags      = 0;
+    if (!r->create(&state.lgrData, lgrParams))
-    if (!(*r)->initialize(&state.lgrData, lgrParams, &sdlFlags))
+      continue;
+
+    // set it's options
+    for(unsigned int i = 0; i < opts->argc; ++i)
+      opts->argv[i].opt->handler(state.lgrData, opts->argv[i].value);
+
+    // initialize the renderer
+    sdlFlags = 0;
+    if (!r->initialize(state.lgrData, &sdlFlags))
     {
-      (*r)->deinitialize(state.lgrData);
+      r->deinitialize(state.lgrData);
       continue;
     }
 
-    state.lgr = *r;
+    state.lgr = r;
-    DEBUG_INFO("Initialized %s", (*r)->get_name());
+    DEBUG_INFO("Initialized %s", r->get_name());
     break;
   }
 
@@ -939,10 +949,8 @@ void doHelp(char * app)
     "  -j        Disable cursor position scaling\n"
     "  -M        Don't hide the host cursor\n"
     "\n"
-    "  -m        Disable mipmapping\n"
-    "  -v        Disable VSYNC\n"
     "  -k        Enable FPS display\n"
-    "  -o FLAG   Specify a renderer flag\n"
+    "  -o FLAG   Specify a renderer option (ie: opengl:vsync=0)\n"
     "\n"
     "  -a        Auto resize the window to the guest\n"
     "  -n        Don't allow the window to be manually resized\n"
@@ -996,7 +1004,7 @@ void doLicense()
 int main(int argc, char * argv[])
 {
   int c;
-  while((c = getopt(argc, argv, "hf:sc:p:jMmvko:anrdFx:y:w:b:Ql")) != -1)
+  while((c = getopt(argc, argv, "hf:sc:p:jMvko:anrdFx:y:w:b:Ql")) != -1)
     switch(c)
     {
       case '?':
@@ -1029,19 +1037,79 @@ int main(int argc, char * argv[])
         params.hideMouse = false;
         break;
 
-      case 'm':
-        params.useMipmap = false;
-        break;
-
-      case 'v':
-        params.vsync = false;
-        break;
-
       case 'k':
         params.showFPS = true;
         break;
 
       case 'o':
+      {
+        const LG_Renderer  * renderer = NULL;
+        RendererOpts       * opts     = NULL;
+
+        const size_t len  = strlen(optarg);
+        const char * name = strtok(optarg, ":");
+
+        for(unsigned int i = 0; i < LG_RENDERER_COUNT; ++i)
+          if (strcasecmp(LG_Renderers[i]->get_name(), name) == 0)
+          {
+            renderer = LG_Renderers[i];
+            opts     = &params.rendererOpts[i];
+            break;
+          }
+
+        if (!renderer)
+        {
+          fprintf(stderr, "No such renderer: %s\n", name);
+          doHelp(argv[0]);
+          return -1;
+        }
+
+        const char * option = strtok(NULL  , "=");
+        if (!option)
+        {
+          fprintf(stderr, "Renderer option name not specified\n");
+          doHelp(argv[0]);
+          return -1;
+        }
+
+        const LG_RendererOpt * opt = NULL;
+        for(unsigned int i = 0; i < renderer->option_count; ++i)
+          if (strcasecmp(option, renderer->options[i].name) == 0)
+          {
+            opt = &renderer->options[i];
+            break;
+          }
+
+        if (!opt)
+        {
+          fprintf(stderr, "Renderer \"%s\" doesn't have the option: %s\n", renderer->get_name(), option);
+          doHelp(argv[0]);
+          return -1;
+        }
+
+        const char * value = NULL;
+        if (len > strlen(name) + strlen(option) + 2)
+          value = option + strlen(option) + 1;
+
+        if (opt->validator && !opt->validator(value))
+        {
+          fprintf(stderr, "Renderer \"%s\" reported Invalid value for option \"%s\"\n", renderer->get_name(), option);
+          doHelp(argv[0]);
+          return -1;
+        }
+
+        if (opts->argc == opts->size)
+        {
+          opts->size += 5;
+          opts->argv  = realloc(opts->argv, sizeof(LG_RendererOptValue) * opts->size);
+        }
+
+        opts->argv[opts->argc].opt   = opt;
+        opts->argv[opts->argc].value = value;
+        ++opts->argc;
+        break;
+      }
+#if 0
         if (params.rendererOptCount == params.rendererOptSize)
         {
           params.rendererOptSize += 5;
@@ -1050,6 +1118,7 @@ int main(int argc, char * argv[])
             params.rendererOptSize * sizeof(char *));
         }
         params.rendererOpts[params.rendererOptCount++] = optarg;
+#endif
         break;
 
       case 'a':

client/renderers/opengl.c

@@ -43,9 +43,23 @@ Place, Suite 330, Boston, MA 02111-1307 USA
 static PFNGLXGETVIDEOSYNCSGIPROC  glXGetVideoSyncSGI  = NULL;
 static PFNGLXWAITVIDEOSYNCSGIPROC glXWaitVideoSyncSGI = NULL;
 
+struct Options
+{
+  bool mipmap;
+  bool vsync;
+};
+
+static struct Options defaultOptions =
+{
+  .mipmap = true,
+  .vsync  = true
+};
+
 struct LGR_OpenGL
 {
   LG_RendererParams params;
+  struct Options    opt;
+
   bool              configured;
   SDL_Window      * sdlWindow;
   SDL_GLContext     glContext;
@@ -104,7 +118,7 @@ const char * lgr_opengl_get_name()
   return "OpenGL";
 }
 
-bool lgr_opengl_initialize(void ** opaque, const LG_RendererParams params, Uint32 * sdlFlags)
+bool lgr_opengl_create(void ** opaque, const LG_RendererParams params)
 {
   // create our local storage
   *opaque = malloc(sizeof(struct LGR_OpenGL));
@@ -116,7 +130,17 @@ bool lgr_opengl_initialize(void ** opaque, const LG_RendererParams params, Uint3
   memset(*opaque, 0, sizeof(struct LGR_OpenGL));
 
   struct LGR_OpenGL * this = (struct LGR_OpenGL *)*opaque;
-  memcpy(&this->params, &params, sizeof(LG_RendererParams));
+  memcpy(&this->params, &params        , sizeof(LG_RendererParams));
+  memcpy(&this->opt   , &defaultOptions, sizeof(struct Options   ));
+
+  return true;
+}
+
+bool lgr_opengl_initialize(void * opaque, Uint32 * sdlFlags)
+{
+  struct LGR_OpenGL * this = (struct LGR_OpenGL *)opaque;
+  if (!this)
+    return false;
 
   if (!glXGetVideoSyncSGI)
   {
@@ -629,7 +653,7 @@ bool lgr_opengl_on_frame_event(void * opaque, const uint8_t * data)
   // unbind the buffer
   glBindBuffer(GL_PIXEL_UNPACK_BUFFER, 0);
 
-  const bool mipmap = this->params.resample && (
+  const bool mipmap = this->opt.mipmap && (
     (this->format.width  > this->destRect.w) ||
     (this->format.height > this->destRect.h));
 
@@ -756,9 +780,36 @@ bool lgr_opengl_render(void * opaque)
   return true;
 }
 
+static void handle_opt_mipmap(void * opaque, const char *value)
+{
+  struct LGR_OpenGL * this = (struct LGR_OpenGL *)opaque;
+  if (!this)
+    return;
+
+  this->opt.mipmap = LG_RendererValueToBool(value);
+}
+
+static void handle_opt_vsync(void * opaque, const char *value)
+{
+  struct LGR_OpenGL * this = (struct LGR_OpenGL *)opaque;
+  if (!this)
+    return;
+
+  this->opt.vsync = LG_RendererValueToBool(value);
+}
+
+static LG_RendererOpt lgr_opengl_options[] =
+{
+  { .name = "mipmap", .validator = LG_RendererValidatorBool, .handler = handle_opt_mipmap },
+  { .name = "vsync" , .validator = LG_RendererValidatorBool, .handler = handle_opt_vsync  }
+};
+
 const LG_Renderer LGR_OpenGL =
 {
   .get_name       = lgr_opengl_get_name,
+  .options        = lgr_opengl_options,
+  .option_count   = LGR_OPTION_COUNT(lgr_opengl_options),
+  .create         = lgr_opengl_create,
   .initialize     = lgr_opengl_initialize,
   .configure      = lgr_opengl_configure,
   .deconfigure    = lgr_opengl_deconfigure,