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

[Samuel Pitoiset]

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?


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