On 02/17/2016 04:27 PM, Manolova, Plamena wrote:
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.


LGTM and passes all the tests, nice work!

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

    On Wed, Feb 17, 2016 at 8:49 AM, Plamena Manolova
    <plamena.manol...@intel.com <mailto:plamena.manol...@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.pa...@intel.com
    <mailto:tapani.pa...@intel.com>>
    > Signed-off-by: Plamena Manolova <plamena.manol...@intel.com
    <mailto:plamena.manol...@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 <imir...@alum.mit.edu
    <mailto:imir...@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




_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to