[Mesa-dev] [PATCH] mesa: change 'SHADER_SUBST' facility to work with env variables
Brian Paul
brianp at vmware.com
Tue Sep 1 06:40:51 PDT 2015
On 09/01/2015 06:04 AM, Tapani Pälli wrote:
>
>
> On 08/31/2015 05:56 PM, Brian Paul wrote:
>> Looks good. Just some minor nitpicks below.
>>
>> Reviewed-by: Brian Paul <brianp at vmware.com>
>
> Thanks! I have one comment below ..
>
>>
>> On 08/31/2015 01:23 AM, Tapani Pälli wrote:
>>> Patch modifies existing shader source and replace functionality to work
>>> with environment variables rather than enable dumping on compile time.
>>> Also instead of _mesa_str_checksum, _mesa_sha1_compute is used to avoid
>>> collisions.
>>>
>>> Functionality is controlled via 2 environment variables:
>>
>> s/2/two/
>>
>>
>>>
>>> MESA_SHADER_DUMP_PATH - path where shader sources are dumped
>>> MESA_SHADER_READ_PATH - path where replacement shaders are read
>>>
>>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>>> Suggested-by: Eero Tamminen <eero.t.tamminen at intel.com>
>>> ---
>>> docs/shading.html | 9 +++
>>> src/mesa/main/shaderapi.c | 136
>>> ++++++++++++++++++++++++++++++++--------------
>>> 2 files changed, 105 insertions(+), 40 deletions(-)
>>>
>>> diff --git a/docs/shading.html b/docs/shading.html
>>> index 77a0ee4..784329e 100644
>>> --- a/docs/shading.html
>>> +++ b/docs/shading.html
>>> @@ -63,6 +63,15 @@ execution. These are generally used for debugging.
>>> Example: export MESA_GLSL=dump,nopt
>>> </p>
>>>
>>> +<p>
>>> +Shaders can be dumped and replaced on runtime for debugging purposes.
>>> +This is controlled via following environment variables:
>>> +<ul>
>>> +<li><b>MESA_SHADER_DUMP_PATH</b> - path where shader sources are dumped
>>> +<li><b>MESA_SHADER_READ_PATH</b> - path where replacement shaders are
>>> read
>>
>> Do these directories need to be different to prevent collisions between
>> the dumped shaders and the replacement shaders?
>
> Yes, I thought a bit of solutions to work only in one dir but I think
> having 2 makes everything more clear and here user can be only one
> overwriting edited contents. I could add some text here : "These paths
> should be separate to avoid overwriting modified contents." (?)
How about "When both are set, these paths should be different so the
dumped shaders do not clobber the replacement shaders."
-Brian
>
>>
>>> +</ul>
>>> +Note, path set must exist before running for dumping or replacing to
>>> work.
>>> +</p>
>>>
>>> <h2 id="support">GLSL Version</h2>
>>>
>>> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
>>> index 0e0e0d6..d3abfc9 100644
>>> --- a/src/mesa/main/shaderapi.c
>>> +++ b/src/mesa/main/shaderapi.c
>>> @@ -53,15 +53,13 @@
>>> #include "program/prog_parameter.h"
>>> #include "util/ralloc.h"
>>> #include "util/hash_table.h"
>>> +#include "util/mesa-sha1.h"
>>> #include <stdbool.h>
>>> #include "../glsl/glsl_parser_extras.h"
>>> #include "../glsl/ir.h"
>>> #include "../glsl/ir_uniform.h"
>>> #include "../glsl/program.h"
>>>
>>> -/** Define this to enable shader substitution (see below) */
>>> -#define SHADER_SUBST 0
>>> -
>>>
>>> /**
>>> * Return mask of GLSL_x flags by examining the MESA_GLSL env var.
>>> @@ -1512,24 +1510,101 @@ _mesa_LinkProgram(GLhandleARB programObj)
>>> link_program(ctx, programObj);
>>> }
>>>
>>> +/**
>>> + * 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 void
>>> +construct_name(const gl_shader_stage stage, const char *source,
>>> + const char *path, char *name, unsigned length)
>>> +{
>>> + char sha[64];
>>> + static const char *types[] = {
>>> + "VS", "TC", "TE", "GS", "FS", "CS",
>>> + };
>>>
>>> + generate_sha1(source, sha);
>>> + _mesa_snprintf(name, length, "%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)
>>> +{
>>> + char name[PATH_MAX];
>>> + static bool path_exists = true;
>>> + char *dump_path;
>>> + FILE *f;
>>> +
>>> + if (!path_exists)
>>> + return NULL;
>>> +
>>> + dump_path = getenv("MESA_SHADER_DUMP_PATH");
>>> +
>>
>> Could remove that empty line.
>>
>>> + if (!dump_path) {
>>> + path_exists = false;
>>> + return NULL;
>>> + }
>>> +
>>> + construct_name(stage, source, dump_path, name, PATH_MAX);
>>> +
>>> + 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", name);
>>> + }
>>> +}
>>>
>>> /**
>>> * Read shader source code from a file.
>>> * Useful for debugging to override an app's shader.
>>> */
>>> static GLcharARB *
>>> -read_shader(const char *fname)
>>> +read_shader(const gl_shader_stage stage, const char *source)
>>> {
>>> - int shader_size = 0;
>>> - FILE *f = fopen(fname, "r");
>>> - GLcharARB *buffer, *shader;
>>> - int len;
>>> + char name[PATH_MAX];
>>> + char *read_path;
>>> + static bool path_exists = true;
>>> + int len, shader_size = 0;
>>> + GLcharARB *buffer;
>>> + FILE *f;
>>>
>>> - if (!f) {
>>> + if (!path_exists)
>>> + return NULL;
>>> +
>>> + read_path = getenv("MESA_SHADER_READ_PATH");
>>> +
>>> + if (!read_path) {
>>> + path_exists = false;
>>> return NULL;
>>> }
>>>
>>> + construct_name(stage, source, read_path, name, PATH_MAX);
>>> +
>>> + f = fopen(name, "r");
>>> +
>>
>> Could remove that empty line.
>>
>>> + if (!f)
>>> + return NULL;
>>> +
>>> /* allocate enough room for the entire shader */
>>> fseek(f, 0, SEEK_END);
>>> shader_size = ftell(f);
>>> @@ -1547,13 +1622,9 @@ read_shader(const char *fname)
>>>
>>> fclose(f);
>>>
>>> - shader = strdup(buffer);
>>> - free(buffer);
>>> -
>>> - return shader;
>>> + return buffer;
>>> }
>>>
>>> -
>>> /**
>>> * Called via glShaderSource() and glShaderSourceARB() API functions.
>>> * Basically, concatenate the source code strings into one long string
>>> @@ -1566,14 +1637,15 @@ _mesa_ShaderSource(GLhandleARB shaderObj,
>>> GLsizei count,
>>> GET_CURRENT_CONTEXT(ctx);
>>> GLint *offsets;
>>> GLsizei i, totalLength;
>>> - GLcharARB *source;
>>> - GLuint checksum;
>>> + GLcharARB *source, *replacement;
>>>
>>> if (!shaderObj || string == NULL) {
>>> _mesa_error(ctx, GL_INVALID_VALUE, "glShaderSourceARB");
>>> return;
>>> }
>>>
>>> + struct gl_shader *sh = _mesa_lookup_shader(ctx, shaderObj);
>>
>> For C sources, I think we're still trying to declare all variables at
>> the top of scope.
>>
>>
>>> +
>>> /*
>>> * This array holds offsets of where the appropriate string ends,
>>> thus the
>>> * last element will be set to the total length of the source code.
>>> @@ -1620,35 +1692,19 @@ _mesa_ShaderSource(GLhandleARB shaderObj,
>>> GLsizei count,
>>> source[totalLength - 1] = '\0';
>>> source[totalLength - 2] = '\0';
>>>
>>> - if (SHADER_SUBST) {
>>> - /* Compute the shader's source code checksum then try to open a
>>> file
>>> - * named newshader_<CHECKSUM>. If it exists, use it in place
>>> of the
>>> - * original shader source code. For debugging.
>>> - */
>>> - char filename[100];
>>> - GLcharARB *newSource;
>>> -
>>> - checksum = _mesa_str_checksum(source);
>>> -
>>> - _mesa_snprintf(filename, sizeof(filename), "newshader_%d",
>>> checksum);
>>> + /* 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);
>>>
>>> - newSource = read_shader(filename);
>>> - if (newSource) {
>>> - fprintf(stderr, "Mesa: Replacing shader %u chksum=%d with
>>> %s\n",
>>> - shaderObj, checksum, filename);
>>> - free(source);
>>> - source = newSource;
>>> - }
>>> + replacement = read_shader(sh->Stage, source);
>>> + if (replacement) {
>>> + free(source);
>>> + source = replacement;
>>> }
>>>
>>> shader_source(ctx, shaderObj, source);
>>>
>>> - if (SHADER_SUBST) {
>>> - struct gl_shader *sh = _mesa_lookup_shader(ctx, shaderObj);
>>> - if (sh)
>>> - sh->SourceChecksum = checksum; /* save original checksum */
>>> - }
>>> -
>>> free(offsets);
>>> }
>>>
>>>
>>
More information about the mesa-dev
mailing list