On 08/21/2015 04:30 PM, Brian Paul wrote:
On Fri, Aug 21, 2015 at 2:17 AM, Tapani Pälli <tapani.pa...@intel.com
<mailto: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
Wow, I've managed to miss this :) I could modify your SHADER_SUBST code
by adding environment variable support and use SHA1 to avoid collisions.
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.
ok
Signed-off-by: Tapani Pälli <tapani.pa...@intel.com
<mailto:tapani.pa...@intel.com>>
Suggested-by: Eero Tamminen <eero.t.tammi...@intel.com
<mailto: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.
agreed, I think I'll just modify your existing code
@@ -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?
ok
+{
+ 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.
ok
+ 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.
will use your code with the checks!
+ 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()?
Yep, I was lazy with this because _mesa_warning would need additional
changes to Makefiles and linking.
+ }
+}
+
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.
Sure, not really needed.
-Brian
struct gl_program *Program; /**< Post-compile assembly code */
GLchar *InfoLog;
--
2.4.3
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org <mailto: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
