[Mesa-dev] [PATCH] compiler/glsl: Fix uniform location counting.

[Manolova, Plamena]

Thank you for reviewing. The change has been run through both Piglit and
CTS. I'll ask Tapani to take another look just in case.

On Wed, Feb 17, 2016 at 4:20 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:

> On Wed, Feb 17, 2016 at 8:49 AM, Plamena Manolova
> <plamena.manolova at intel.com> wrote:
> > This patch moves the calculation of current uniforms to
> > link_uniforms, which makes use of UniformRemapTable which
> > stores all the reserved uniform locations.
> >
> > Location assignment for implicit uniforms now tries to use
> > any gaps left in the table after the location assignment
> > for explicit uniforms. This gives us more space to store more
> > uniforms.
> >
> > Patch is based on earlier patch with following changes/additions:
> >
> >    1: Move the counting of explicit locations to
> >       check_explicit_uniform_locations and then pass
> >       the number to link_assign_uniform_locations.
> >    2: Count the number of empty slots in UniformRemapTable
> >       and store them in a list_head.
> >    3: Try to find an empty slot for implicit locations from
> >       the list, if that fails resize UniformRemapTable.
> >
> > Fixes following CTS tests:
> >    ES31-CTS.explicit_uniform_location.uniform-loc-mix-with-implicit-max
> >
> ES31-CTS.explicit_uniform_location.uniform-loc-mix-with-implicit-max-array
> >
> > Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> > Signed-off-by: Plamena Manolova <plamena.manolova at intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93696
> > ---
> >  src/compiler/glsl/link_uniforms.cpp | 80
> ++++++++++++++++++++++++++++++++-----
> >  src/compiler/glsl/linker.cpp        | 70
> +++++++++++++++++++++-----------
> >  src/compiler/glsl/linker.h          | 17 +++++++-
> >  src/mesa/main/mtypes.h              |  8 ++++
> >  4 files changed, 140 insertions(+), 35 deletions(-)
> >
> > diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> > index bad1c17..5326bfd 100644
> > --- a/src/compiler/glsl/linker.cpp
> > +++ b/src/compiler/glsl/linker.cpp
> > @@ -4129,6 +4148,7 @@ link_shaders(struct gl_context *ctx, struct
> gl_shader_program *prog)
> >
> >     tfeedback_decl *tfeedback_decls = NULL;
> >     unsigned num_tfeedback_decls = prog->TransformFeedback.NumVarying;
> > +   unsigned int num_explicit_uniform_locs = 0;
>
> This initializer seems unnecessary, since you initialize it
> unconditionally further down. Otherwise this change is
>
> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
>
> I assume you've also run this through piglit (if not, please do). It
> may be good for Tapani to have another look at this since he
> originally tried to implement this and is likely more aware of the
> various pitfalls.
>
> Cheers,
>
>   -ilia
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160217/a610a792/attachment.html>