[Mesa-dev] [PATCH 10/10] glsl/linker: Pack between varyings.

[Paul Berry]

On 12 December 2012 14:06, Eric Anholt <eric at anholt.net> wrote:

> Paul Berry <stereotype441 at gmail.com> writes:
>
> > This patch implements varying packing between varyings.
> >
> > Previously, each varying occupied components 0 through N-1 of its
> > assigned varying slot, so there was no way to pack two varyings into
> > the same slot.  For example, if the varyings were a float, a vec2, a
> > vec3, and another vec2, they would be stored as follows:
> >
> >  <----slot1----> <----slot2----> <----slot3----> <----slot4---->  slots
> >   *   *   *   *   *   *   *   *   *   *   *   *   *   *   *   *
> >  flt  x   x   x  <vec2->  x   x  <--vec3--->  x  <vec3->  x   x
> varyings
>
> That's a funny-looking vec3 you've got here.       ^^^^^^^
>

Heh, oops.


>
> > -      if (producer_var->type->is_array()) {
> > -         const unsigned slots = producer_var->type->length
> > -            * producer_var->type->fields.array->matrix_columns;
> > +      /* Advance to the next slot if this varying has a different
> packing
> > +       * class than the previous one, and we're not already on a slot
> > +       * boundary.
> > +       */
> > +      if (i > 0 && generic_location % 4 != 0 &&
> > +          this->matches[i - 1].packing_class
> > +          != this->matches[i].packing_class) {
> > +         generic_location += 4 - generic_location % 4;
>
> Grab an align macro from anywhere else (we really need to just stuff
> something in macros.h), then "generic_location = ALIGN(generic_location,
> 4)", and you can skip checking for already being on a slot boundary.
>

Ok, seems reasonable.


>
> >  /**
> > + * Compute the number of components that this variable will occupy when
> > + * properly packed.
> > + */
> > +unsigned
> > +varying_matches::compute_num_components(ir_variable *var)
> > +{
> > +   const glsl_type *type = var->type;
> > +   unsigned multipiler = 1;
> > +
> > +   if (type->is_array()) {
> > +      multipiler *= type->length;
> > +      type = type->fields.array;
> > +   }
> > +
> > +   /* FINISHME: Support for "varying" records in GLSL 1.50. */
> > +   assert(!type->is_record());
> > +
> > +   return multipiler * type->components();
>
> "multiplier"
>
> I think this function is just type->component_slots(), though, at the
> expense of using a different meaning of the term "slot" from the rest of
> the code you're working on.
>

Wow, I could have sworn I looked for that function and didn't find it :)

Thanks for your review, Eric.  I'll incorporate your comments into v2 as
soon as I can.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20121212/26e91fae/attachment-0001.html>