On 12 December 2012 14:06, Eric Anholt <span dir="ltr"><<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> writes:<br>
<br>
> This patch implements varying packing between varyings.<br>
><br>
> Previously, each varying occupied components 0 through N-1 of its<br>
> assigned varying slot, so there was no way to pack two varyings into<br>
> the same slot.  For example, if the varyings were a float, a vec2, a<br>
> vec3, and another vec2, they would be stored as follows:<br>
><br>
>  <----slot1----> <----slot2----> <----slot3----> <----slot4---->  slots<br>
>   *   *   *   *   *   *   *   *   *   *   *   *   *   *   *   *<br>
>  flt  x   x   x  <vec2->  x   x  <--vec3--->  x  <vec3->  x   x   varyings<br>
<br>
</div>That's a funny-looking vec3 you've got here.       ^^^^^^^<br></blockquote><div><br>Heh, oops.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div class="im"><br>
> -      if (producer_var->type->is_array()) {<br>
> -         const unsigned slots = producer_var->type->length<br>
> -            * producer_var->type->fields.array->matrix_columns;<br>
> +      /* Advance to the next slot if this varying has a different packing<br>
> +       * class than the previous one, and we're not already on a slot<br>
> +       * boundary.<br>
> +       */<br>
> +      if (i > 0 && generic_location % 4 != 0 &&<br>
> +          this->matches[i - 1].packing_class<br>
> +          != this->matches[i].packing_class) {<br>
> +         generic_location += 4 - generic_location % 4;<br>
<br>
</div>Grab an align macro from anywhere else (we really need to just stuff<br>
something in macros.h), then "generic_location = ALIGN(generic_location,<br>
4)", and you can skip checking for already being on a slot boundary.<br></blockquote><div><br>Ok, seems reasonable.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div class="im"><br>
>  /**<br>
> + * Compute the number of components that this variable will occupy when<br>
> + * properly packed.<br>
> + */<br>
> +unsigned<br>
> +varying_matches::compute_num_components(ir_variable *var)<br>
> +{<br>
> +   const glsl_type *type = var->type;<br>
> +   unsigned multipiler = 1;<br>
> +<br>
> +   if (type->is_array()) {<br>
> +      multipiler *= type->length;<br>
> +      type = type->fields.array;<br>
> +   }<br>
> +<br>
> +   /* FINISHME: Support for "varying" records in GLSL 1.50. */<br>
> +   assert(!type->is_record());<br>
> +<br>
> +   return multipiler * type->components();<br>
<br>
</div>"multiplier"<br>
<br>
I think this function is just type->component_slots(), though, at the<br>
expense of using a different meaning of the term "slot" from the rest of<br>
the code you're working on.<br>
</blockquote></div><br>Wow, I could have sworn I looked for that function and didn't find it :)<br><br>Thanks for your review, Eric.  I'll incorporate your comments into v2 as soon as I can.<br></div>