[Mesa-dev] [PATCH] mesa/main: Rework the shader capture naming convention

Benedikt Schemmer ben at besd.de
Sat Apr 28 14:39:07 UTC 2018


Change from a purely number.shader_test naming scheme to sha_number.shader_test
because especially games often use the same number for entirely different shaders
based on graphics settings etc. and then already captured shaders get overwritten.
It is also useful for capturing shaders from applications that reuse program
numbers a lot i.e. piglit. This has helped to identify problems there as well.

Plus minor cleanups, that could be split into a separate patch.
I have used this for approximately two months now, seems to work just fine.

---
 src/mesa/main/shaderapi.c | 315 ++++++++++++++++++++++++----------------------
 1 file changed, 168 insertions(+), 147 deletions(-)

diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 44b18af492..65cf0a67a2 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -64,6 +64,122 @@
 #include "util/mesa-sha1.h"
 #include "util/crc32.h"

+
+/**
+ * Generate a SHA-1 hash value string for given source string.
+ */
+static void
+generate_sha1(const char *source, char sha_str[64])
+{
+   unsigned char sha[20];
+   _mesa_sha1_compute(source, strlen(source), sha);
+   _mesa_sha1_format(sha_str, sha);
+}
+
+
+#ifdef ENABLE_SHADER_CACHE
+/**
+ * Construct a full path for shader replacement functionality using
+ * following format:
+ *
+ * <path>/<stage prefix>_<CHECKSUM>.glsl
+ */
+static char *
+construct_name(const gl_shader_stage stage, const char *source,
+               const char *path)
+{
+   char sha[64];
+   static const char *types[] = {
+      "VS", "TC", "TE", "GS", "FS", "CS",
+   };
+
+   generate_sha1(source, sha);
+   return ralloc_asprintf(NULL, "%s/%s_%s.glsl", path, types[stage], sha);
+}
+
+/**
+ * Write given shader source to a file in MESA_SHADER_DUMP_PATH.
+ */
+static void
+dump_shader(const gl_shader_stage stage, const char *source)
+{
+   static bool path_exists = true;
+   char *dump_path;
+   FILE *f;
+
+   if (!path_exists)
+      return;
+
+   dump_path = getenv("MESA_SHADER_DUMP_PATH");
+   if (!dump_path) {
+      path_exists = false;
+      return;
+   }
+
+   char *name = construct_name(stage, source, dump_path);
+
+   f = fopen(name, "w");
+   if (f) {
+      fputs(source, f);
+      fclose(f);
+   } else {
+      GET_CURRENT_CONTEXT(ctx);
+      _mesa_warning(ctx, "could not open %s for dumping shader (%s)", name,
+                    strerror(errno));
+   }
+   ralloc_free(name);
+}
+
+/**
+ * Read shader source code from a file.
+ * Useful for debugging to override an app's shader.
+ */
+static GLcharARB *
+read_shader(const gl_shader_stage stage, const char *source)
+{
+   char *read_path;
+   static bool path_exists = true;
+   int len, shader_size = 0;
+   GLcharARB *buffer;
+   FILE *f;
+
+   if (!path_exists)
+      return NULL;
+
+   read_path = getenv("MESA_SHADER_READ_PATH");
+   if (!read_path) {
+      path_exists = false;
+      return NULL;
+   }
+
+   char *name = construct_name(stage, source, read_path);
+   f = fopen(name, "r");
+   ralloc_free(name);
+   if (!f)
+      return NULL;
+
+   /* allocate enough room for the entire shader */
+   fseek(f, 0, SEEK_END);
+   shader_size = ftell(f);
+   rewind(f);
+   assert(shader_size);
+
+   /* add one for terminating zero */
+   shader_size++;
+
+   buffer = malloc(shader_size);
+   assert(buffer);
+
+   len = fread(buffer, 1, shader_size, f);
+   buffer[len] = 0;
+
+   fclose(f);
+
+   return buffer;
+}
+
+#endif /* ENABLE_SHADER_CACHE */
+
 /**
  * Return mask of GLSL_x flags by examining the MESA_GLSL env var.
  */
@@ -125,11 +241,10 @@ _mesa_init_shader_state(struct gl_context *ctx)
    /* Device drivers may override these to control what kind of instructions
     * are generated by the GLSL compiler.
     */
-   struct gl_shader_compiler_options options;
+   struct gl_shader_compiler_options options = {};
    gl_shader_stage sh;
    int i;

-   memset(&options, 0, sizeof(options));
    options.MaxUnrollIterations = 32;
    options.MaxIfDepth = UINT_MAX;

@@ -138,7 +253,7 @@ _mesa_init_shader_state(struct gl_context *ctx)

    ctx->Shader.Flags = _mesa_get_shader_flags();

-   if (ctx->Shader.Flags != 0)
+   if (ctx->Shader.Flags)
       ctx->Const.GenerateTemporaryNames = true;

    /* Extended for ARB_separate_shader_objects */
@@ -655,7 +770,8 @@ get_programiv(struct gl_context *ctx, GLuint program, GLenum pname,
               GLint *params)
 {
    struct gl_shader_program *shProg
-      = _mesa_lookup_shader_program_err(ctx, program, "glGetProgramiv(program)");
+      = _mesa_lookup_shader_program_err(ctx, program,
+                                        "glGetProgramiv(program)");

    /* Is transform feedback available in this context?
     */
@@ -837,7 +953,7 @@ get_programiv(struct gl_context *ctx, GLuint program, GLenum pname,
       *params = shProg->BinaryRetreivableHint;
       return;
    case GL_PROGRAM_BINARY_LENGTH:
-      if (ctx->Const.NumProgramBinaryFormats == 0) {
+      if (!ctx->Const.NumProgramBinaryFormats) {
          *params = 0;
       } else {
          _mesa_get_program_binary_length(ctx, shProg, params);
@@ -858,7 +974,7 @@ get_programiv(struct gl_context *ctx, GLuint program, GLenum pname,
                      "linked)");
          return;
       }
-      if (shProg->_LinkedShaders[MESA_SHADER_COMPUTE] == NULL) {
+      if (!shProg->_LinkedShaders[MESA_SHADER_COMPUTE]) {
          _mesa_error(ctx, GL_INVALID_OPERATION, "glGetProgramiv(no compute "
                      "shaders)");
          return;
@@ -1118,7 +1234,7 @@ _mesa_compile_shader(struct gl_context *ctx, struct gl_shader *sh)
    } else {
       if (ctx->_Shader->Flags & GLSL_DUMP) {
          _mesa_log("GLSL source for %s shader %d:\n",
-                 _mesa_shader_stage_to_string(sh->Stage), sh->Name);
+                   _mesa_shader_stage_to_string(sh->Stage), sh->Name);
          _mesa_log("%s\n", sh->Source);
       }

@@ -1227,29 +1343,46 @@ link_program(struct gl_context *ctx, struct gl_shader_program *shProg,
    /* Capture .shader_test files. */
    const char *capture_path = _mesa_get_shader_capture_path();
    if (shProg->Name != 0 && shProg->Name != ~0 && capture_path != NULL) {
+
       FILE *file;
-      char *filename = ralloc_asprintf(NULL, "%s/%u.shader_test",
-                                       capture_path, shProg->Name);
+      char *filename = NULL;
+      char *fsource = NULL;
+      char *ftemp = NULL;
+
+      asprintf(&fsource, "[require]\nGLSL%s >= %u.%02u\n",
+               shProg->IsES ? " ES" : "",
+               shProg->data->Version / 100, shProg->data->Version % 100);
+
+      if (shProg->SeparateShader) {
+         ftemp = fsource;
+         asprintf(&fsource, "%sGL_ARB_separate_shader_objects\nSSO ENABLED\n",
+                  ftemp);
+         free(ftemp);
+      }
+
+      for (unsigned i = 0; i < shProg->NumShaders; i++) {
+          ftemp = fsource;
+          asprintf(&fsource, "%s\n[%s shader]\n%s\n", ftemp,
+                   _mesa_shader_stage_to_string(shProg->Shaders[i]->Stage),
+                   shProg->Shaders[i]->Source);
+          free(ftemp);
+      }
+
+      char shabuf[64] = {"mylittlebunny"};
+      generate_sha1(fsource, shabuf);
+
+      asprintf(&filename, "%s/%s_%u.shader_test", capture_path,
+               shabuf, shProg->Name);
       file = fopen(filename, "w");
       if (file) {
-         fprintf(file, "[require]\nGLSL%s >= %u.%02u\n",
-                 shProg->IsES ? " ES" : "",
-                 shProg->data->Version / 100, shProg->data->Version % 100);
-         if (shProg->SeparateShader)
-            fprintf(file, "GL_ARB_separate_shader_objects\nSSO ENABLED\n");
-         fprintf(file, "\n");
-
-         for (unsigned i = 0; i < shProg->NumShaders; i++) {
-            fprintf(file, "[%s shader]\n%s\n",
-                    _mesa_shader_stage_to_string(shProg->Shaders[i]->Stage),
-                    shProg->Shaders[i]->Source);
-         }
+         fprintf(file, "%s", fsource);
          fclose(file);
       } else {
          _mesa_warning(ctx, "Failed to open %s", filename);
       }

-      ralloc_free(filename);
+      free(filename);
+      free(fsource);
    }

    if (shProg->data->LinkStatus == LINKING_FAILURE &&
@@ -1265,13 +1398,13 @@ link_program(struct gl_context *ctx, struct gl_shader_program *shProg,
       GLuint i;

       printf("Link %u shaders in program %u: %s\n",
-                   shProg->NumShaders, shProg->Name,
-                   shProg->data->LinkStatus ? "Success" : "Failed");
+             shProg->NumShaders, shProg->Name,
+             shProg->data->LinkStatus ? "Success" : "Failed");

       for (i = 0; i < shProg->NumShaders; i++) {
          printf(" shader %u, stage %u\n",
-                      shProg->Shaders[i]->Name,
-                      shProg->Shaders[i]->Stage);
+                shProg->Shaders[i]->Name,
+                shProg->Shaders[i]->Stage);
       }
    }
 }
@@ -1344,7 +1477,7 @@ void
 _mesa_active_program(struct gl_context *ctx, struct gl_shader_program *shProg,
 		     const char *caller)
 {
-   if ((shProg != NULL) && !shProg->data->LinkStatus) {
+   if ((shProg) && !shProg->data->LinkStatus) {
       _mesa_error(ctx, GL_INVALID_OPERATION,
 		  "%s(program %u not linked)", caller, shProg->Name);
       return;
@@ -1678,7 +1811,7 @@ void GLAPIENTRY
 _mesa_GetObjectParameterfvARB(GLhandleARB object, GLenum pname,
                               GLfloat *params)
 {
-   GLint iparams[1] = {0};  /* XXX is one element enough? */
+   GLint iparams[1] = {};  /* XXX is one element enough? */
    _mesa_GetObjectParameterivARB(object, pname, iparams);
    params[0] = (GLfloat) iparams[0];
 }
@@ -1775,119 +1908,7 @@ _mesa_LinkProgram(GLuint programObj)
    link_program_error(ctx, shProg);
 }

-#ifdef ENABLE_SHADER_CACHE
-/**
- * Generate a SHA-1 hash value string for given source string.
- */
-static void
-generate_sha1(const char *source, char sha_str[64])
-{
-   unsigned char sha[20];
-   _mesa_sha1_compute(source, strlen(source), sha);
-   _mesa_sha1_format(sha_str, sha);
-}
-
-/**
- * Construct a full path for shader replacement functionality using
- * following format:
- *
- * <path>/<stage prefix>_<CHECKSUM>.glsl
- */
-static char *
-construct_name(const gl_shader_stage stage, const char *source,
-               const char *path)
-{
-   char sha[64];
-   static const char *types[] = {
-      "VS", "TC", "TE", "GS", "FS", "CS",
-   };
-
-   generate_sha1(source, sha);
-   return ralloc_asprintf(NULL, "%s/%s_%s.glsl", path, types[stage], sha);
-}
-
-/**
- * Write given shader source to a file in MESA_SHADER_DUMP_PATH.
- */
-static void
-dump_shader(const gl_shader_stage stage, const char *source)
-{
-   static bool path_exists = true;
-   char *dump_path;
-   FILE *f;
-
-   if (!path_exists)
-      return;
-
-   dump_path = getenv("MESA_SHADER_DUMP_PATH");
-   if (!dump_path) {
-      path_exists = false;
-      return;
-   }
-
-   char *name = construct_name(stage, source, dump_path);
-
-   f = fopen(name, "w");
-   if (f) {
-      fputs(source, f);
-      fclose(f);
-   } else {
-      GET_CURRENT_CONTEXT(ctx);
-      _mesa_warning(ctx, "could not open %s for dumping shader (%s)", name,
-                    strerror(errno));
-   }
-   ralloc_free(name);
-}
-
-/**
- * Read shader source code from a file.
- * Useful for debugging to override an app's shader.
- */
-static GLcharARB *
-read_shader(const gl_shader_stage stage, const char *source)
-{
-   char *read_path;
-   static bool path_exists = true;
-   int len, shader_size = 0;
-   GLcharARB *buffer;
-   FILE *f;
-
-   if (!path_exists)
-      return NULL;

-   read_path = getenv("MESA_SHADER_READ_PATH");
-   if (!read_path) {
-      path_exists = false;
-      return NULL;
-   }
-
-   char *name = construct_name(stage, source, read_path);
-   f = fopen(name, "r");
-   ralloc_free(name);
-   if (!f)
-      return NULL;
-
-   /* allocate enough room for the entire shader */
-   fseek(f, 0, SEEK_END);
-   shader_size = ftell(f);
-   rewind(f);
-   assert(shader_size);
-
-   /* add one for terminating zero */
-   shader_size++;
-
-   buffer = malloc(shader_size);
-   assert(buffer);
-
-   len = fread(buffer, 1, shader_size, f);
-   buffer[len] = 0;
-
-   fclose(f);
-
-   return buffer;
-}
-
-#endif /* ENABLE_SHADER_CACHE */

 /**
  * Called via glShaderSource() and glShaderSourceARB() API functions.
@@ -1908,7 +1929,7 @@ shader_source(struct gl_context *ctx, GLuint shaderObj, GLsizei count,
       if (!sh)
          return;

-      if (string == NULL) {
+      if (!string) {
          _mesa_error(ctx, GL_INVALID_VALUE, "glShaderSourceARB");
          return;
       }
@@ -1921,7 +1942,7 @@ shader_source(struct gl_context *ctx, GLuint shaderObj, GLsizei count,
     * last element will be set to the total length of the source code.
     */
    offsets = malloc(count * sizeof(GLint));
-   if (offsets == NULL) {
+   if (!offsets) {
       _mesa_error(ctx, GL_OUT_OF_MEMORY, "glShaderSourceARB");
       return;
    }
@@ -1948,7 +1969,7 @@ shader_source(struct gl_context *ctx, GLuint shaderObj, GLsizei count,
     */
    totalLength = offsets[count - 1] + 2;
    source = malloc(totalLength * sizeof(GLcharARB));
-   if (source == NULL) {
+   if (!source) {
       free((GLvoid *) offsets);
       _mesa_error(ctx, GL_OUT_OF_MEMORY, "glShaderSourceARB");
       return;
@@ -2241,7 +2262,7 @@ _mesa_GetProgramBinary(GLuint program, GLsizei bufSize, GLsizei *length,
     * Ensure that length always points to valid storage to avoid multiple NULL
     * pointer checks below.
     */
-   if (length == NULL)
+   if (!length)
       length = &length_dummy;


@@ -2259,7 +2280,7 @@ _mesa_GetProgramBinary(GLuint program, GLsizei bufSize, GLsizei *length,
       return;
    }

-   if (ctx->Const.NumProgramBinaryFormats == 0) {
+   if (!ctx->Const.NumProgramBinaryFormats) {
       *length = 0;
       _mesa_error(ctx, GL_INVALID_OPERATION,
                   "glGetProgramBinary(driver supports zero binary formats)");
@@ -2854,7 +2875,7 @@ _mesa_UniformSubroutinesuiv(GLenum shadertype, GLsizei count,
    bool flushed = false;
    do {
       struct gl_uniform_storage *uni = p->sh.SubroutineUniformRemapTable[i];
-      if (uni == NULL) {
+      if (!uni) {
          i++;
          continue;
       }
@@ -3050,7 +3071,7 @@ _mesa_shader_write_subroutine_index(struct gl_context *ctx,
 {
    int i, j;

-   if (p->sh.NumSubroutineUniformRemapTable == 0)
+   if (!p->sh.NumSubroutineUniformRemapTable)
       return;

    i = 0;
-- 
2.14.1


More information about the mesa-dev mailing list