[Mesa-dev] [v2 6/6] mesa: OES_get_program_binary functionality

[Tapani Pälli]

On 11/01/2013 12:21 PM, Erik Faye-Lund wrote:
> On Fri, Nov 1, 2013 at 10:16 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/mesa/main/shaderapi.c | 46 +++++++++++++++++++++++++++++++++++++++-------
>>   1 file changed, 39 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
>> index 7da860d..67aee6b 100644
>> --- a/src/mesa/main/shaderapi.c
>> +++ b/src/mesa/main/shaderapi.c
>> @@ -57,6 +57,8 @@
>>   #include "../glsl/ir.h"
>>   #include "../glsl/ir_uniform.h"
>>   #include "../glsl/program.h"
>> +#include "../glsl/ir_cache.h"
>> +#include "git_sha1.h"
>>
>>   /** Define this to enable shader substitution (see below) */
>>   #define SHADER_SUBST 0
>> @@ -212,7 +214,6 @@ attach_shader(struct gl_context *ctx, GLuint program, GLuint shader)
>>      struct gl_shader_program *shProg;
>>      struct gl_shader *sh;
>>      GLuint i, n;
>> -
>>      const bool same_type_disallowed = _mesa_is_gles(ctx);
>>
>>      shProg = _mesa_lookup_shader_program_err(ctx, program, "glAttachShader");
>> @@ -1630,8 +1631,26 @@ _mesa_GetProgramBinary(GLuint program, GLsizei bufSize, GLsizei *length,
>>      if (length != NULL)
>>         *length = 0;
>>
>> -   (void) binaryFormat;
>> -   (void) binary;
>> +   size_t size = 0;
>> +   char *data = _mesa_program_serialize(shProg, &size, MESA_GIT_SHA1);
>> +
>> +   /* we have more data that can fit to user given buffer */
>> +   if (size > bufSize) {
>> +      _mesa_error(ctx, GL_INVALID_OPERATION, __FUNCTION__);
>> +      if (data)
>> +         free(data);
>> +      return;
>> +   }
>> +
>> +   if (data) {
>> +      memcpy(binary, data, size);
>> +      free(data);
>> +   }
>> +
>> +   if (length != NULL)
>> +      *length = size;
>> +
>> +   *binaryFormat = 0;
>>   }
>>
>>   void GLAPIENTRY
>> @@ -1645,10 +1664,23 @@ _mesa_ProgramBinary(GLuint program, GLenum binaryFormat,
>>      if (!shProg)
>>         return;
>>
>> -   (void) binaryFormat;
>> -   (void) binary;
>> -   (void) length;
>> -   _mesa_error(ctx, GL_INVALID_OPERATION, __FUNCTION__);
>> +   if (length <= 0)
>> +      return;
>> +
>> +   /* free possible existing data and initialize structure */
>> +   _mesa_free_shader_program_data(ctx, shProg);
>> +   _mesa_init_shader_program(ctx, shProg);
>> +
>> +   /* fill structure from a binary blob */
>> +   if (_mesa_program_deserialize(shProg, binary, length, MESA_GIT_SHA1)) {
>
> Won't using the git-sha1 as a compatibility-criteria cause issues for
> developers with local changes? I'm not so worried about this for
> OES_get_program_binary itself, but once the shader-cache is in place
> it sounds like a potential source of difficult to track down
> misbehavior...

I agree it might be too aggressive criteria but it is hard to come up 
with better and as simple. Putting versions or hard coding sizes of the 
required structures feels too intrusive for me although it would prevent 
cache usage only when something really changed. Maybe some lighter 
version scheme where version is kept to the IR as a whole (?) For 
failures the specification states:

"An implementation may reject a program binary if it determines the 
program binary was produced by an incompatible or outdated version of 
the compiler.In this case the application should fall back to providing 
the original OpenGL Shading Language source shaders, and perhaps again 
retrieve the program binary for future use."

(http://www.khronos.org/registry/gles/extensions/OES/OES_get_program_binary.txt)

// Tapani