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

[Samuel Pitoiset]

On 05/04/2017 09:44 PM, Nicolai Hähnle wrote:
> On 04.05.2017 20:54, Marek Olšák wrote:
>> On Wed, Apr 26, 2017 at 9:40 AM, Nicolai Hähnle <nhaehnle at gmail.com> 
>> 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.
>>
>> I don't like this. Are you suggesting that a texture bound to unit 5
>> has to be somehow converted to a bindless texture and made resident
>> under the hood (in Mesa), so that the bindless shader code works?
> 
> I don't think we have much of a choice, unfortunately. I agree it's very 
> annoying.
> 
> This can only happen for default block uniforms, which st/mesa uploads 
> in a special way anyway. Maybe we can also treat default block bindless 
> uniforms in a special way?
> 
> I don't think this case has been fully implemented in the WIP branches.

Well, that's really unfortunate. The spec also says:

"For samplers and images with values specified as texture image or image 
units, the GL implemenation will translate the unit number to an 
internal handle as required."

If my understanding is correct, we need to a way to ask the driver and 
get a handle to a bound texture?

Yeah, that part is not implemented mainly because it's... really annoying.

> 
> Cheers,
> Nicolai