Thanks for reviewing!
Am 17.05.2018 um 08:42 schrieb Tapani Pälli:
>
>
> On 05/10/2018 12:05 PM, Benedikt Schemmer wrote:
>> Move shader-cache code from back to front and make generate_sha1() usable
>> unconditionally to avoid code duplication in the following patch
>>
>> ---
>> src/mesa/main/shaderapi.c | 228
>> +++++++++++++++++++++++-----------------------
>> 1 file changed, 116 insertions(+), 112 deletions(-)
>>
>> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
>> index 44b18af492..e8acca4490 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);
>> +}
>
> There is one potential problem here. The 'ENABLE_SHADER_CACHE' guard for
> generate_sha1 and others was placed there because the imported sha1 code
> broke windows build, I'm wondering if this is still
> the case? If so, then generate_sha1 should be inside ENABLE_SHADER_CACHE
> guard.
>
I did a quick
gedit $(grep -Rlsi "_mesa_sha1_compute" | grep -E "\.c|\.h")
and it seems radv and anv use _mesa_sha1_compute (and _mesa_sha1_format)
without a guard
best example from Intel seems to be brw_disk_cache.c which uses it alot outside
of the ENABLE_SHADER_CACHE guard
so probably safe?
>> +
>> +
>> +#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.
>> */
>> @@ -1775,119 +1891,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.
>>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
