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

[Samuel Pitoiset]

On 05/04/2017 09:55 PM, Samuel Pitoiset wrote:
> 
> 
> On 05/04/2017 09:51 PM, Samuel Pitoiset wrote:
>>
>>
>> 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?
> 
> If doing this makes sense, we can consider all uniform samplers declared 
> with the bindless_sampler layout qualifier to be a constant value at the 
> TGSI level.
> 
> And when glUniform1i() is setting a value to a bindless sampler, we do 
> something like: ctx->Driver.GetTextureHandleFromUnit(x) and put the 
> 64-bit value into the uniform storage?

No, more complicated actually.

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