[Mesa-dev] [PATCH v2 23/31] glsl: allow bindless samplers/images to be assigned

[Samuel Pitoiset]

On 04/26/2017 09:54 AM, Nicolai Hähnle wrote:
> On 24.04.2017 12:35, Samuel Pitoiset wrote:
>> The ARB_bindless_texture allows sampler and image types to be
>> declared as temporary variables, to be converted from/to uvec2
>> using explicit constructors, so they can be assigned.
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>  src/compiler/glsl/ast_to_hir.cpp | 21 ++++++++++++++++++---
>>  1 file changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/compiler/glsl/ast_to_hir.cpp 
>> b/src/compiler/glsl/ast_to_hir.cpp
>> index e4d8bae2f5..8ad6211e2d 100644
>> --- a/src/compiler/glsl/ast_to_hir.cpp
>> +++ b/src/compiler/glsl/ast_to_hir.cpp
>> @@ -940,14 +940,29 @@ do_assignment(exec_list *instructions, struct 
>> _mesa_glsl_parse_state *state,
>>        lhs_var->data.assigned = true;
>>
>>     if (!error_emitted) {
>> +      bool is_read_only = lhs_var != NULL && (lhs_var->data.read_only ||
>> +                          (lhs_var->data.mode == 
>> ir_var_shader_storage &&
>> +                           lhs_var->data.image_read_only));
>> +
>> +      if (state->has_bindless()) {
>> +         if ((lhs_var->type->is_sampler() || 
>> lhs_var->type->is_image()) &&
>> +             (lhs_var->data.bindless || lhs_var->data.mode != 
>> ir_var_uniform)) {
>> +            /* A sampler (or an image) is considered bindless either 
>> if it has
>> +             * been declared in the default uniform block with the
>> +             * bindless_sampler (respectively bindless_image) layout 
>> qualifier,
>> +             * or when its storage is not uniform because this is not 
>> allowed
>> +             * without ARB_bindless_texture.
>> +             */
>> +            is_read_only = false;
>> +         }
>> +      }
> 
> I don't understand why this is necessary. I think the correct approach 
> is not to set lhs_var->data.read_only for sampler and image variables.

That could be a different approach, I will a second look at this patch.

> 
> Cheers,
> Nicolai
> 
> 
>> +
>>        if (non_lvalue_description != NULL) {
>>           _mesa_glsl_error(&lhs_loc, state,
>>                            "assignment to %s",
>>                            non_lvalue_description);
>>           error_emitted = true;
>> -      } else if (lhs_var != NULL && (lhs_var->data.read_only ||
>> -                 (lhs_var->data.mode == ir_var_shader_storage &&
>> -                  lhs_var->data.image_read_only))) {
>> +      } else if (is_read_only) {
>>           /* We can have image_read_only set on both images and buffer 
>> variables,
>>            * but in the former there is a distinction between 
>> assignments to
>>            * the variable itself (read_only) and to the memory they 
>> point to
>>
> 
>