[Mesa-dev] [PATCH 1/7] linker: Split set_uniform_binding into separate functions for blocks and samplers

[Kenneth Graunke]

On 04/04/2014 02:01 PM, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
> 
> The two code paths are quite different, and there are some problems in
> the handling of uniform blocks.  Future changes will cause these paths
> to diverge further.  Ultimately, selecting between the two functions
> will happen at the set_uniform_binding call site, and
> set_uniform_binding will be deleted.
> 
> NOTE: This patch just moves code around.
> 
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76323
> Cc: "10.1" <mesa-stable at lists.freedesktop.org>
> Cc: github at socker.lepus.uberspace.de
> ---
>  src/glsl/link_uniform_initializers.cpp | 42 +++++++++++++++++++++++++++++++---
>  1 file changed, 39 insertions(+), 3 deletions(-)

Assuming you have a reasonable response to my comment on patch 5, this
series is:

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

though, I'm not sure how much that's worth - I had to re-read the GLSL
rules and re-discover how our compiler IR for this stuff works.  The
code seems right, but I could be totally missing something obvious.

On that note...is it just me, or is the compiler IR for uniform blocks
rather ugly and messy?

Anyway, thanks a ton for doing this, Ian.  Sorry for dropping the ball
when we first implemented 420pack.

> diff --git a/src/glsl/link_uniform_initializers.cpp b/src/glsl/link_uniform_initializers.cpp
> index 9d6977d..9a10350 100644
> --- a/src/glsl/link_uniform_initializers.cpp
> +++ b/src/glsl/link_uniform_initializers.cpp
> @@ -84,7 +84,7 @@ copy_constant_to_storage(union gl_constant_value *storage,
>  }
>  
>  void
> -set_uniform_binding(void *mem_ctx, gl_shader_program *prog,
> +set_sampler_binding(void *mem_ctx, gl_shader_program *prog,
>                      const char *name, const glsl_type *type, int binding)
>  {
>     struct gl_uniform_storage *const storage =
> @@ -95,7 +95,7 @@ set_uniform_binding(void *mem_ctx, gl_shader_program *prog,
>        return;
>     }
>  
> -   if (storage->type->is_sampler()) {
> +   {
>        unsigned elements = MAX2(storage->array_elements, 1);
>  
>        /* From section 4.4.4 of the GLSL 4.20 specification:
> @@ -118,7 +118,24 @@ set_uniform_binding(void *mem_ctx, gl_shader_program *prog,
>              }
>           }
>        }
> -   } else if (storage->block_index != -1) {
> +   }
> +
> +   storage->initialized = true;
> +}
> +
> +void
> +set_block_binding(void *mem_ctx, gl_shader_program *prog,
> +                  const char *name, const glsl_type *type, int binding)
> +{
> +   struct gl_uniform_storage *const storage =
> +      get_storage(prog->UniformStorage, prog->NumUserUniformStorage, name);
> +
> +   if (storage == NULL) {
> +      assert(storage != NULL);
> +      return;
> +   }
> +
> +   if (storage->block_index != -1) {
>        /* This is a field of a UBO.  val is the binding index. */
>        for (int i = 0; i < MESA_SHADER_STAGES; i++) {
>           int stage_index = prog->UniformBlockStageIndex[i][storage->block_index];
> @@ -134,6 +151,25 @@ set_uniform_binding(void *mem_ctx, gl_shader_program *prog,
>  }
>  
>  void
> +set_uniform_binding(void *mem_ctx, gl_shader_program *prog,
> +                    const char *name, const glsl_type *type, int binding)
> +{
> +   struct gl_uniform_storage *const storage =
> +      get_storage(prog->UniformStorage, prog->NumUserUniformStorage, name);
> +
> +   if (storage == NULL) {
> +      assert(storage != NULL);
> +      return;
> +   }
> +
> +   if (storage->type->is_sampler()) {
> +      set_sampler_binding(mem_ctx, prog, name, type, binding);
> +   } else if (storage->block_index != -1) {
> +      set_block_binding(mem_ctx, prog, name, type, binding);
> +   }
> +}
> +
> +void
>  set_uniform_initializer(void *mem_ctx, gl_shader_program *prog,
>  			const char *name, const glsl_type *type,
>  			ir_constant *val)
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140409/6919e54a/attachment.sig>