[Mesa-dev] [PATCH] mesa: shader dump/read support for ARB programs

[Alejandro Piñeiro]

On 14/08/18 10:58, Tapani Pälli wrote:
>
>
> On 08/14/2018 11:53 AM, Alejandro Piñeiro wrote:
>>
>>
>> On 24/07/18 09:41, Tapani Pälli wrote:
>>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106283
>>> ---
>>>   src/mesa/main/arbprogram.c | 26 ++++++++++++++++++++++++++
>>>   src/mesa/main/shaderapi.c  | 17 ++++++++++-------
>>>   src/mesa/main/shaderapi.h  |  6 ++++++
>>>   3 files changed, 42 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/src/mesa/main/arbprogram.c b/src/mesa/main/arbprogram.c
>>> index b169bce0c5..d1dc5029a9 100644
>>> --- a/src/mesa/main/arbprogram.c
>>> +++ b/src/mesa/main/arbprogram.c
>>> @@ -326,6 +326,19 @@ get_env_param_pointer(struct gl_context *ctx,
>>> const char *func,
>>>      }
>>>   }
>>>   +static gl_shader_stage
>>> +stage_from_target(GLenum target)
>>
>> How about using _mesa_program_enum_to_shader_stage? As far as I see it
>> is defined at both main/shaderobj.h and program/program.h, so hopefully
>> one of those could be used.
>
> Thanks, I did not realize there was this helper. I'll change the code
> to use that instead! I guess r-b holds with that?

Yes, the rb still holds. Sorry, I forgot the usual "with or without it"

>
>> One wild guess is that you want to be sure that only
>> VERTEX_PROGRAM/FRAGMENT_PROGRAM is used. If that is the case, ignore my
>> comment.
>>
>> Reviewed-by: Alejandro Piñeiro <apinheiro at igalia.com>
>>
>>> +{
>>> +   switch (target) {
>>> +   case GL_VERTEX_PROGRAM_ARB:
>>> +      return MESA_SHADER_VERTEX;
>>> +   case GL_FRAGMENT_PROGRAM_ARB:
>>> +      return MESA_SHADER_FRAGMENT;
>>> +   default:
>>> +      return -1;
>>> +   }
>>> +}
>>> +
>>>   void GLAPIENTRY
>>>   _mesa_ProgramStringARB(GLenum target, GLenum format, GLsizei len,
>>>                          const GLvoid *string)
>>> @@ -347,6 +360,19 @@ _mesa_ProgramStringARB(GLenum target, GLenum
>>> format, GLsizei len,
>>>         return;
>>>      }
>>>   +#ifdef ENABLE_SHADER_CACHE
>>> +   GLcharARB *replacement;
>>> +
>>> +   /* Dump original shader source to MESA_SHADER_DUMP_PATH and replace
>>> +    * if corresponding entry found from MESA_SHADER_READ_PATH.
>>> +    */
>>> +   _mesa_dump_shader_source(stage_from_target(target), string);
>>> +
>>> +   replacement =
>>> _mesa_read_shader_source(stage_from_target(target), string);
>>> +   if (replacement)
>>> +      string = replacement;
>>> +#endif /* ENABLE_SHADER_CACHE */
>>> +
>>>      if (target == GL_VERTEX_PROGRAM_ARB &&
>>> ctx->Extensions.ARB_vertex_program) {
>>>         prog = ctx->VertexProgram.Current;
>>>         _mesa_parse_arb_vertex_program(ctx, target, string, len, prog);
>>> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
>>> index f7080847cc..2ea8d965ab 100644
>>> --- a/src/mesa/main/shaderapi.c
>>> +++ b/src/mesa/main/shaderapi.c
>>> @@ -1795,6 +1795,7 @@ generate_sha1(const char *source, char
>>> sha_str[64])
>>>    * following format:
>>>    *
>>>    * <path>/<stage prefix>_<CHECKSUM>.glsl
>>> + * <path>/<stage prefix>_<CHECKSUM>.arb
>>>    */
>>>   static char *
>>>   construct_name(const gl_shader_stage stage, const char *source,
>>> @@ -1805,15 +1806,17 @@ construct_name(const gl_shader_stage stage,
>>> const char *source,
>>>         "VS", "TC", "TE", "GS", "FS", "CS",
>>>      };
>>>   +   const char *format = strncmp(source, "!!ARB", 5) ? "glsl" :
>>> "arb";
>>> +
>>>      generate_sha1(source, sha);
>>> -   return ralloc_asprintf(NULL, "%s/%s_%s.glsl", path,
>>> types[stage], sha);
>>> +   return ralloc_asprintf(NULL, "%s/%s_%s.%s", path, types[stage],
>>> sha, format);
>>>   }
>>>     /**
>>>    * Write given shader source to a file in MESA_SHADER_DUMP_PATH.
>>>    */
>>> -static void
>>> -dump_shader(const gl_shader_stage stage, const char *source)
>>> +void
>>> +_mesa_dump_shader_source(const gl_shader_stage stage, const char
>>> *source)
>>>   {
>>>      static bool path_exists = true;
>>>      char *dump_path;
>>> @@ -1846,8 +1849,8 @@ dump_shader(const gl_shader_stage stage, const
>>> char *source)
>>>    * 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)
>>> +GLcharARB *
>>> +_mesa_read_shader_source(const gl_shader_stage stage, const char
>>> *source)
>>>   {
>>>      char *read_path;
>>>      static bool path_exists = true;
>>> @@ -1971,9 +1974,9 @@ shader_source(struct gl_context *ctx, GLuint
>>> shaderObj, GLsizei count,
>>>      /* Dump original shader source to MESA_SHADER_DUMP_PATH and
>>> replace
>>>       * if corresponding entry found from MESA_SHADER_READ_PATH.
>>>       */
>>> -   dump_shader(sh->Stage, source);
>>> +   _mesa_dump_shader_source(sh->Stage, source);
>>>   -   replacement = read_shader(sh->Stage, source);
>>> +   replacement = _mesa_read_shader_source(sh->Stage, source);
>>>      if (replacement) {
>>>         free(source);
>>>         source = replacement;
>>> diff --git a/src/mesa/main/shaderapi.h b/src/mesa/main/shaderapi.h
>>> index dbfd68fd3a..a8227ecc96 100644
>>> --- a/src/mesa/main/shaderapi.h
>>> +++ b/src/mesa/main/shaderapi.h
>>> @@ -374,6 +374,12 @@ extern GLvoid GLAPIENTRY
>>>   _mesa_GetProgramStageiv(GLuint program, GLenum shadertype,
>>>                           GLenum pname, GLint *values);
>>>   +GLcharARB *
>>> +_mesa_read_shader_source(const gl_shader_stage stage, const char
>>> *source);
>>> +
>>> +void
>>> +_mesa_dump_shader_source(const gl_shader_stage stage, const char
>>> *source);
>>> +
>>>   #ifdef __cplusplus
>>>   }
>>>   #endif
>>
>>
>