[Mesa-dev] [PATCH] freedreno/ir3: Handle GL_NONE in get_num_components_for_glformat()

[Eduardo Lima Mitev]

On 12/19/18 9:23 PM, Ilia Mirkin wrote:
> On Wed, Dec 19, 2018 at 3:18 AM Eduardo Lima Mitev <elima at igalia.com> wrote:
>>
>> An earlier patch that introduced the function failed to handle the case
>> where an image format layout qualifier is not specified, which is allowed
>> in Core profiles. In these cases, nir_variable's image format is
>> GL_NONE, and we don't need to print a debug message for those.
>> ---
>>  src/freedreno/ir3/ir3_compiler_nir.c | 11 ++++++++---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/freedreno/ir3/ir3_compiler_nir.c b/src/freedreno/ir3/ir3_compiler_nir.c
>> index 19aef3eb27e..4b309fb3c84 100644
>> --- a/src/freedreno/ir3/ir3_compiler_nir.c
>> +++ b/src/freedreno/ir3/ir3_compiler_nir.c
>> @@ -1306,11 +1306,16 @@ get_num_components_for_glformat(GLuint format)
>>         case GL_RGB10_A2:
>>                 return 4;
>>
>> +       case GL_NONE:
>> +               /* Omitting the image format qualifier is allowed on GL profiles.
> 
> I realize you're trying to make the distinction against GL ES, but
> that may not be clear. You could say "... on desktop GL profiles", for
> example. Your call. Either way,
> 

Yeah, I use GL vs. GLES profiles to make that distinction, but agree it
might not be clear outside my head, so I will clarify the comment before
pushing.

Thanks!

> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
> 
>> +                * Assuming 4 components is always safe.
>> +                */
>> +               return 4;
>> +
>>         default:
>>                 /* Return 4 components also for all other formats we don't know
>> -                * about. This is always safe. Also, the format should have been
>> -                * validated already by the higher level API. Drop a debug message
>> -                * just in case.
>> +                * about. The format should have been validated already by
>> +                * the higher level API, but drop a debug message just in case.
>>                  */
>>                 debug_printf("Unhandled GL format %u while emitting imageStore()\n",
>>                                          format);
>> --
>> 2.19.2
>>
>