[Mesa-dev] [PATCH v2 19/31] glsl: fix explicit binding location for bindless samplers/images

[Samuel Pitoiset]

On 04/26/2017 09:40 AM, Nicolai Hähnle wrote:
> On 24.04.2017 12:35, Samuel Pitoiset wrote:
>> The ARB_bindless_texture spec says:
>>
>>    "Interactions with GLSL 4.20"
>>
>>    "Without GLSL 4.20 support, sampler and image uniforms may only
>>     be initialized through the OpenGL API. With GLSL 4.20, sampler
>>     and image uniforms may be initialized in the shader using
>>
>>     layout(binding = integer-constant)
>>
>>     as described in section 4.4.4 "Opaque-Uniform Layout Qualifiers".
>>     When ARB_bindless_texture is supported, these initial binding
>>     values are always taken to mean a texture image or image unit
>>     number, not a bindless handle."
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>  src/compiler/glsl/ast_to_hir.cpp | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/src/compiler/glsl/ast_to_hir.cpp 
>> b/src/compiler/glsl/ast_to_hir.cpp
>> index ae81ed95d8..943c25a224 100644
>> --- a/src/compiler/glsl/ast_to_hir.cpp
>> +++ b/src/compiler/glsl/ast_to_hir.cpp
>> @@ -2920,6 +2920,26 @@ apply_explicit_binding(struct 
>> _mesa_glsl_parse_state *state,
>>        return;
>>     }
>>
>> +   /* The ARB_bindless_texture_spec says:
>> +    *
>> +    * "Interactions with GLSL 4.20"
>> +    *
>> +    * "Without GLSL 4.20 support, sampler and image uniforms may only be
>> +    *  initialized through the OpenGL API. With GLSL 4.20, sampler 
>> and image
>> +    *  uniforms may be initialized in the shader using
>> +    *
>> +    *  layout(binding = integer-constant)
>> +    *
>> +    *  as described in section 4.4.4 "Opaque-Uniform Layout 
>> Qualifiers".  When
>> +    *  ARB_bindless_texture is supported, these initial binding 
>> values are
>> +    *  always taken to mean a texture image or image unit number, not a
>> +    *  bindless handle."
>> +    */
>> +   if (state->has_bindless() &&
>> +       (base_type->is_sampler() || base_type->is_image())) {
>> +      var->data.bindless = false;
>> +   }
> 
> I don't think this is correct. The way I understand the spec, saying
> 
>    layout (bindless_sampler, binding = 5) uniform sampler2D s;
> 
> means that s is still bindless, it's just that it's *initial* value is 
> set up as if the application had called
> 
>    glUniform1i(index of s, 5);
> 
> You're still allowed to later on change it to a bindless handle by 
> calling one of the glUniformHandleui64* functions.

After double-checking the spec, yeah that makes sense.

I will drop the patch.

Thanks for noticing this!

> 
> Cheers,
> Nicolai
> 
> 
>> +
>>     var->data.explicit_binding = true;
>>     var->data.binding = qual_binding;
>>
>>
> 
>