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

2018-12-19 Thread 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  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 
> 
>> +* 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
>>
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-19 Thread Ilia Mirkin
On Wed, Dec 19, 2018 at 3:18 AM Eduardo Lima Mitev  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,

Reviewed-by: Ilia Mirkin 

> +* 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
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-19 Thread Eduardo Lima Mitev
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.
+* 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

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev