On Fri, Aug 21, 2015 at 2:17 AM, Tapani Pälli <tapani.pa...@intel.com>
wrote:
> Patch adds shader source and replace functionality in to the compiler.
>
I had some very primitive support for this sort of thing already. See
SHADER_SUBST in shaderapi.c However, feel free to replace it with
something better/nicer.
Also, please document this feature in docs/shading.html
> This can be used to debug individual (failing) shaders and measure
> performance impact of each shader.
>
> Functionality is controlled via 2 environment variables:
>
> MESA_SHADER_DUMP - path where shader sources are dumped
> MESA_SHADER_READ - path where replacement shaders are read
>
Let's put _PATH on the end of both of those to better describe the vars.
>
> Signed-off-by: Tapani Pälli <tapani.pa...@intel.com>
> Suggested-by: Eero Tamminen <eero.t.tammi...@intel.com>
> ---
> src/glsl/glsl_parser_extras.cpp | 76
> +++++++++++++++++++++++++++++++++++++++++
> src/mesa/main/mtypes.h | 2 +-
> 2 files changed, 77 insertions(+), 1 deletion(-)
>
> diff --git a/src/glsl/glsl_parser_extras.cpp
> b/src/glsl/glsl_parser_extras.cpp
> index 46896d7..17b79b0 100644
> --- a/src/glsl/glsl_parser_extras.cpp
> +++ b/src/glsl/glsl_parser_extras.cpp
>
Are you sure this is the right file for this new code? Maybe it should go
into a new source file. glsl_parser_extras.cpp already seems to have
become a dumping ground for a lot of unrelated things.
> @@ -1570,10 +1570,86 @@ set_shader_inout_layout(struct gl_shader *shader,
>
> extern "C" {
>
> +#include <sys/stat.h>
> +#include "util/mesa-sha1.h"
> +
> +static void
> +generate_sha1(struct gl_shader *shader, char sha_str[64])
>
const-qualify shader?
> +{
> + unsigned char sha[20];
> + _mesa_sha1_compute(shader->Source, strlen(shader->Source), sha);
> + _mesa_sha1_format(sha_str, sha);
> +}
> +
> +static void
> +construct_name(struct gl_shader *shader, const char *path,
>
const qualify shader?
Please add a comment to the function explaining what it does.
> + char *name, unsigned length)
> +{
> + char sha[64];
> + static const char *types[] = {
> + "VS", "TC", "TE", "GS", "FS", "CS",
> + };
> +
> + generate_sha1(shader, sha);
> + _mesa_snprintf(name, length, "%s/%s_%s.glsl", path,
> types[shader->Stage],
> + sha);
> +}
> +
> +static void
> +read_shader(struct gl_shader *shader)
>
Please put a comment on the function explaining what it does.
> +{
> + char name[PATH_MAX];
> + static char *read_path = getenv("MESA_SHADER_READ");
> +
> + if (!read_path)
> + return;
> +
> + construct_name(shader, read_path, name, PATH_MAX);
> +
> + FILE *in = fopen(name, "r");
> + if (in) {
> + fseek(in, 0, SEEK_END);
> + long size = ftell(in);
> + rewind(in);
> + char *source = (char *) malloc(size + 1);
>
Should probably check for source==NULL here. Otherwise, tools like
coverity will probably complain.
> + fread(source, size, 1, in);
> + source[size] = '\0';
> + free(shader->Source);
> + shader->Source = source;
> + fclose(in);
> + }
> +}
> +
> +static void
> +dump_shader(struct gl_shader *shader)
>
const-qualify shader?
> +{
> + char name[PATH_MAX];
> + static char *dump_path = getenv("MESA_SHADER_DUMP");
> +
> + if (!dump_path)
> + return;
> +
> + construct_name(shader, dump_path, name, PATH_MAX);
> +
> + FILE *out = fopen(name, "w");
> + if (out) {
> + fprintf(out, "%s", shader->Source);
>
fputs() instead?
> + fclose(out);
> + } else {
> + fprintf(stderr, "could not open %s for dumping shader\n", name);
>
Or, _mesa_warning()?
> + }
> +}
> +
> void
> _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader
> *shader,
> bool dump_ast, bool dump_hir)
> {
> + /* Dump original shader source to MESA_SHADER_DUMP and replace
> + * if corresponding entry found from MESA_SHADER_READ path.
> + */
> + dump_shader(shader);
> + read_shader(shader);
> +
> struct _mesa_glsl_parse_state *state =
> new(shader) _mesa_glsl_parse_state(ctx, shader->Stage, shader);
> const char *source = shader->Source;
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index f61a245..fb47a22 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2346,7 +2346,7 @@ struct gl_shader
> bool IsES; /**< True if this shader uses GLSL ES */
>
> GLuint SourceChecksum; /**< for debug/logging purposes */
> - const GLchar *Source; /**< Source code string */
> + GLchar *Source; /**< Source code string */
>
Not sure why that change is needed.
-Brian
>
> struct gl_program *Program; /**< Post-compile assembly code */
> GLchar *InfoLog;
> --
> 2.4.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
