[Mesa-dev] [PATCH 4/6] glsl: teach opt_structure_splitting about images in structures

[Samuel Pitoiset]

On 05/31/2017 02:39 AM, Timothy Arceri wrote:
> On 26/05/17 04:07, Samuel Pitoiset wrote:
>> GL_ARB_bindless_texture allows images to be declared inside
>> structures, but when memory/format qualifiers are used, they
>> should be propagated when structures are splitted.
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>   src/compiler/glsl/opt_structure_splitting.cpp | 23 
>> +++++++++++++++++++++--
>>   1 file changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/compiler/glsl/opt_structure_splitting.cpp 
>> b/src/compiler/glsl/opt_structure_splitting.cpp
>> index eac98b74cc..fb2120f5fe 100644
>> --- a/src/compiler/glsl/opt_structure_splitting.cpp
>> +++ b/src/compiler/glsl/opt_structure_splitting.cpp
>> @@ -344,11 +344,30 @@ do_structure_splitting(exec_list *instructions)
>>         for (unsigned int i = 0; i < entry->var->type->length; i++) {
>>            const char *name = ralloc_asprintf(mem_ctx, "%s_%s", 
>> entry->var->name,
>>                                               
>> type->fields.structure[i].name);
>> -
>> -         entry->components[i] =
>> +         ir_variable *new_var =
>>               new(entry->mem_ctx) 
>> ir_variable(type->fields.structure[i].type,
>>                                               name,
>>                                               (ir_variable_mode) 
>> entry->var->data.mode);
>> +
>> +         if (type->fields.structure[i].type->contains_image()) {
> 
> Do we need to use contains_image() here rather than 
> without_array()->is_image()?
> 
> Wouldn't contains_image() mean that for a struct that contained a struct 
> which contained an image, we would enter the if unnecessary when 
> splitting the outer struct (not to mention the needless recursion over 
> nested struct in contains_image() itself).
> 
> If you agree with changing this to without_array()->is_image() then:
> 
> Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>

Yeah, it looks more correct.

> 
> 
>> +            /* Do not lost memory/format qualifiers for images 
>> declared inside
>> +             * structures as allowed by ARB_bindless_texture.
>> +             */
>> +            new_var->data.memory_read_only =
>> +               type->fields.structure[i].memory_read_only;
>> +            new_var->data.memory_write_only =
>> +               type->fields.structure[i].memory_write_only;
>> +            new_var->data.memory_coherent =
>> +               type->fields.structure[i].memory_coherent;
>> +            new_var->data.memory_volatile =
>> +               type->fields.structure[i].memory_volatile;
>> +            new_var->data.memory_restrict =
>> +               type->fields.structure[i].memory_restrict;
>> +            new_var->data.image_format =
>> +               type->fields.structure[i].image_format;
>> +         }
>> +
>> +         entry->components[i] = new_var;
>>            entry->var->insert_before(entry->components[i]);
>>         }
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev