[Mesa-dev] [PATCH] TransformFeedback: Assign transform feedback varying slots in linker.

[Dan McCabe]

On 10/23/2011 01:19 PM, Marek Olšák wrote:
> On Sat, Oct 22, 2011 at 2:14 AM, Dan McCabe<zen3d.linux at gmail.com>  wrote:
>> Modify the linker to assign additional slots for varying
>> variables used by transform feedback. This is done after other
>> varyings are already assigned slots.
>>
>> Since this is done after previous varying slot assignments,
>> the code needs to know how many varyings are already assigned
>> slots. A new function "max_varying()" is introduced to examine a
>> previously processed shader to find the largest varying already
>> assigned a slot. All new varyings will be assigned slots after
>> this one. If varyings are found, -1 is returned.
>> ---
>>   src/glsl/linker.cpp |   84 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 files changed, 84 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
>> index a7c38a3..81f2658 100644
>> --- a/src/glsl/linker.cpp
>> +++ b/src/glsl/linker.cpp
>> @@ -207,6 +207,25 @@ invalidate_variable_locations(gl_shader *sh, enum ir_variable_mode mode,
>>     }
>>   }
>>
>> +
>> +int
>> +max_varying(gl_shader *sh, enum ir_variable_mode mode)
>> +{
>> +   int max_varying = -1;
>> +
>> +   foreach_list(node, sh->ir) {
>> +      ir_variable *const var = ((ir_instruction *) node)->as_variable();
>> +
>> +      if ((var == NULL) || (var->mode != (unsigned) mode))
>> +        continue;
>> +
>> +      if (var->location>  max_varying)
>> +        max_varying = var->location;
>> +   }
>> +
>> +   return max_varying;
>> +}
>> +
>>
>>   /**
>>   * Determine the number of attribute slots required for a particular type
>> @@ -1597,6 +1616,69 @@ assign_varying_locations(struct gl_context *ctx,
>>
>>
>>   void
>> +assign_transform_feedback_varying_locations(struct gl_context *ctx,
>> +                                           struct gl_shader_program *prog)
>> +{
>> +   struct gl_shader *vs = prog->_LinkedShaders[MESA_SHADER_VERTEX];
>> +
>> +   if (vs == NULL)
>> +      return;
>> +
>> +   char **names = prog->TransformFeedback.VaryingNames;
>> +   int num_names = prog->TransformFeedback.NumVarying;
>> +
>> +   if (num_names<= 0)
>> +      return;
>> +
>> +   int num_varying = max_varying(vs, ir_var_out) + 1;
>> +   unsigned output_index =
>> +     (num_varying>  VERT_RESULT_VAR0)
>> +        ? num_varying
>> +        : VERT_RESULT_VAR0;
>> +
>> +   foreach_list(node, vs->ir) {
>> +      ir_variable *const output_var = ((ir_instruction *) node)->as_variable();
>> +
>> +      if (output_var == NULL
>> +      ||  output_var->mode != ir_var_out
>> +      ||  output_var->location != -1)
>> +        continue;
>> +
>> +      /* Find a transform feedback varying variable that has
>> +       * the same name as the shader variable.
>> +       */
>> +      int varying_index = -1;
>> +      for (int num_name = 0; num_name<  num_names; num_name++) {
>> +         char *name = *names++;
>
> Hi Dan,
>
> Sorry for the very late reply and thank you very much for the work. I
> have just a few remarks.
>
> 1) *names++ doesn't seem correct and causes a segfault. You probably
> wanted *(names++). I fixed this in my code by using names[num_name],
> which is more readable.

Agreed on both points. Thanks.

>
> 2) I am not sure whether your code will work with varying arrays. With
> "varying vec4 array[2];", "array[0]" and "array[1]" are possible names
> for transform feedback varyings.

Possibly not; this hasn't been tested extensively. But I'll check.

>
> 3) If a transform feedback varying is undeclared, linker_error should
> be called and the linker should back off.

Agreed.

>
> Other than that, transform feedback varying locations seem to be
> assigned correctly.
>
> Best regards,
> Marek