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

[Ian Romanick]

On 04/09/2014 09:10 AM, Kenneth Graunke wrote:
> 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?

Yes.  Part of the problem is that we implemented things one "easy" way
for GL_ARB_uniform_buffer_objects / OpenGL 3.1, but the addition of 3.2
and OpenGL ES 3.0 features made the easy implementation not work.
Rather that completely gut everything, I refactored a bunch of stuff
and, basically, added a parallel implementation for the new bits.

> 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: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140410/7aa1d475/attachment.sig>