[Mesa-dev] [PATCH] glsl: serialize data from glTransformFeedbackVaryings

[Tapani Pälli]

On 06/14/2018 08:46 PM, Jordan Justen wrote:
> On 2018-06-14 02:58:33, Tapani Pälli wrote:
>> While XFB has been enabled for cache, we did not serialize enough
>> data for the whole API to work (such as glGetProgramiv).
>>
>> Fixes: 6d830940f7 "Allow shader cache usage with transform feedback"
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106907
>> ---
>>   src/compiler/glsl/serialize.cpp | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/src/compiler/glsl/serialize.cpp b/src/compiler/glsl/serialize.cpp
>> index 727822633d..4cb74ddba9 100644
>> --- a/src/compiler/glsl/serialize.cpp
>> +++ b/src/compiler/glsl/serialize.cpp
>> @@ -323,6 +323,12 @@ write_xfb(struct blob *metadata, struct gl_shader_program *shProg)
>>   
>>      blob_write_uint32(metadata, prog->info.stage);
>>   
>> +   /* Data set by glTransformFeedbackVaryings. */
>> +   blob_write_uint32(metadata, shProg->TransformFeedback.BufferMode);
>> +   blob_write_uint32(metadata, shProg->TransformFeedback.NumVarying);
>> +   for (unsigned i = 0; i < shProg->TransformFeedback.NumVarying; i++)
>> +      blob_write_string(metadata, shProg->TransformFeedback.VaryingNames[i]);
> 
> I guess we need BufferStride too?

Yep, at first I thought it was something calculated internally but it is 
part of API, I'll add it.

>> +
>>      blob_write_uint32(metadata, ltf->NumOutputs);
>>      blob_write_uint32(metadata, ltf->ActiveBuffers);
>>      blob_write_uint32(metadata, ltf->NumVarying);
>> @@ -352,6 +358,15 @@ read_xfb(struct blob_reader *metadata, struct gl_shader_program *shProg)
>>      if (xfb_stage == ~0u)
>>         return;
>>   
>> +   /* Data set by glTransformFeedbackVaryings. */
>> +   shProg->TransformFeedback.BufferMode = blob_read_uint32(metadata);
>> +   shProg->TransformFeedback.NumVarying = blob_read_uint32(metadata);
>> +   shProg->TransformFeedback.VaryingNames = (char **)
>> +      malloc(shProg->TransformFeedback.NumVarying * sizeof(GLchar *));
>> +   for (unsigned i = 0; i < shProg->TransformFeedback.NumVarying; i++)
>> +      shProg->TransformFeedback.VaryingNames[i] =
>> +         strdup(blob_read_string(metadata));
> 
> I guess VaryingNames uses malloc/free rather than ralloc. Maybe worth
> a comment? A comment might just be extra noise though too.

Yep, that is the case. I'll add "Note, malloc used with VaryingNames.".

> With BufferStride added,
> 
> Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

Thanks!

>> +
>>      struct gl_program *prog = shProg->_LinkedShaders[xfb_stage]->Program;
>>      struct gl_transform_feedback_info *ltf =
>>         rzalloc(prog, struct gl_transform_feedback_info);
>> -- 
>> 2.14.4
>>