[Mesa-dev] [PATCH 09/25] glsl: Add support for image binding qualifiers.

[Francisco Jerez]

Timothy Arceri <t_arceri at yahoo.com.au> writes:

> On Mon, 2015-08-17 at 19:45 +0300, Francisco Jerez wrote:
>> Support for binding an image to an image unit explicitly in the shader
>> source is required by both GLSL 4.2 and GLSL ES 3.1, but not by the
>> original ARB_shader_image_load_store extension.
>> ---
>>  src/glsl/ast_to_hir.cpp                | 12 +++++++++++-
>>  src/glsl/link_uniform_initializers.cpp | 24 +++++++++++++++++-------
>>  2 files changed, 28 insertions(+), 8 deletions(-)
>> 
>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>> index 06cd6a5..0bf7a1f 100644
>> --- a/src/glsl/ast_to_hir.cpp
>> +++ b/src/glsl/ast_to_hir.cpp
>> @@ -2181,10 +2181,20 @@ validate_binding_qualifier(struct 
>> _mesa_glsl_parse_state *state,
>>  
>>           return false;
>>        }
>> +   } else if (state->is_version(420, 310) &&
>> +              var->type->without_array()->is_image()) {
>> +      assert(ctx->Const.MaxImageUnits <= MAX_IMAGE_UNITS);
>> +      if (max_index >= ctx->Const.MaxImageUnits) {
>> +         _mesa_glsl_error(loc, state, "Image binding %d exceeds the "
>> +                          " maximum number of image units (%d)", max_index,
>> +                          ctx->Const.MaxImageUnits);
>> +         return false;
>> +      }
>> +
>>     } else {
>>        _mesa_glsl_error(loc, state,
>>                         "the \"binding\" qualifier only applies to uniform "
>> -                       "blocks, samplers, atomic counters, or arrays 
>> thereof");
>> +                       "blocks, opaque variables, or arrays thereof");
>>        return false;
>>     }
>>  
>> diff --git a/src/glsl/link_uniform_initializers.cpp 
>> b/src/glsl/link_uniform_initializers.cpp
>> index d61ae91..7c6269b 100644
>> --- a/src/glsl/link_uniform_initializers.cpp
>> +++ b/src/glsl/link_uniform_initializers.cpp
>> @@ -101,7 +101,7 @@ copy_constant_to_storage(union gl_constant_value 
>> *storage,
>>  }
>>  
>>  void
>> -set_sampler_binding(gl_shader_program *prog, const char *name, int binding)
>> +set_opaque_binding(gl_shader_program *prog, const char *name, int binding)
>
> I guess you could add a comment the atomics are handled elsewhere but not a
> big deal.
>
> Reviewed-by: Timothy Arceri <t_arceri at yahoo.com.au>
>

| --- a/src/glsl/link_uniform_initializers.cpp
| +++ b/src/glsl/link_uniform_initializers.cpp
| @@ -100,6 +100,11 @@ copy_constant_to_storage(union gl_constant_value *storage,
|     }
|  }
| 
| +/**
| + * Initialize an opaque uniform from the value of an explicit binding
| + * qualifier specified in the shader.  Atomic counters are different because
| + * they have no storage and should be handled elsewhere.
| + */
|  void
|  set_opaque_binding(gl_shader_program *prog, const char *name, int binding)
|  {

Does that look okay for you?

Thanks.

>
>>  {
>>     struct gl_uniform_storage *const storage =
>>        get_storage(prog->UniformStorage, prog->NumUniformStorage, name);
>> @@ -127,11 +127,20 @@ set_sampler_binding(gl_shader_program *prog, const 
>> char *name, int binding)
>>     for (int sh = 0; sh < MESA_SHADER_STAGES; sh++) {
>>        gl_shader *shader = prog->_LinkedShaders[sh];
>>  
>> -      if (shader && storage->sampler[sh].active) {
>> -         for (unsigned i = 0; i < elements; i++) {
>> -            unsigned index = storage->sampler[sh].index + i;
>> +      if (shader) {
>> +         if (storage->type->base_type == GLSL_TYPE_SAMPLER &&
>> +             storage->sampler[sh].active) {
>> +            for (unsigned i = 0; i < elements; i++) {
>> +               const unsigned index = storage->sampler[sh].index + i;
>> +               shader->SamplerUnits[index] = storage->storage[i].i;
>> +            }
>>  
>> -            shader->SamplerUnits[index] = storage->storage[i].i;
>> +         } else if (storage->type->base_type == GLSL_TYPE_IMAGE &&
>> +                    storage->image[sh].active) {
>> +            for (unsigned i = 0; i < elements; i++) {
>> +               const unsigned index = storage->image[sh].index + i;
>> +               shader->ImageUnits[index] = storage->storage[i].i;
>> +            }
>>           }
>>        }
>>     }
>> @@ -267,8 +276,9 @@ link_set_uniform_initializers(struct gl_shader_program 
>> *prog,
>>           if (var->data.explicit_binding) {
>>              const glsl_type *const type = var->type;
>>  
>> -            if (type->without_array()->is_sampler()) {
>> -               linker::set_sampler_binding(prog, var->name, var
>> ->data.binding);
>> +            if (type->without_array()->is_sampler() ||
>> +                type->without_array()->is_image()) {
>> +               linker::set_opaque_binding(prog, var->name, var
>> ->data.binding);
>>              } else if (var->is_in_buffer_block()) {
>>                 const glsl_type *const iface_type = var
>> ->get_interface_type();
>>  
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150819/54dc3667/attachment.sig>