[Mesa-dev] [PATCH v2 08/14] mesa: handle OES_texture_half_float formats in _mesa_base_tex_format()

[Tapani Pälli]

On 05/04/2018 10:15 AM, Christian Gmeiner wrote:
> Hi
> 
> 
>> On 05/01/2018 05:48 PM, Christian Gmeiner wrote:
>>> Signed-off-by: Christian Gmeiner <christian.gmeiner at gmail.com>
>>> Reviewed-by: Wladimir J. van der Laan <laanwj at gmail.com>
>>> ---
>>>    src/mesa/main/glformats.c | 19 +++++++++++++++++++
>>>    1 file changed, 19 insertions(+)
>>>
>>> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
>>> index cba5e670db..1d3d524875 100644
>>> --- a/src/mesa/main/glformats.c
>>> +++ b/src/mesa/main/glformats.c
>>> @@ -2424,6 +2424,25 @@ _mesa_base_tex_format(const struct gl_context
> *ctx, GLint internalFormat)
>>>             return GL_YCBCR_MESA;
>>>       }
>>>
>>> +   if (ctx->Extensions.OES_texture_half_float) {
> 
>> this could be
> 
>> if ((ctx->Extensions.ARB_texture_float) ||
>>      ctx->Extensions.OES_texture_half_float))
> 
> 
> I tried to separate half float and float handling so I think this your
> suggestion will
> not work here.
> 
>>> +       switch (internalFormat) {
>>> +       case GL_ALPHA16F_ARB:
>>> +          return GL_ALPHA;
>>> +       case GL_RGBA16F_ARB:
>>> +          return GL_RGBA;
>>> +       case GL_RGB16F_ARB:
>>> +          return GL_RGB;
>>> +       case GL_INTENSITY16F_ARB:
>>> +          return GL_INTENSITY;
>>> +       case GL_LUMINANCE16F_ARB:
>>> +          return GL_LUMINANCE;
>>> +       case GL_LUMINANCE_ALPHA16F_ARB:
>>> +          return GL_LUMINANCE_ALPHA;
>>> +       default:
>>> +          ; /* fallthrough */
>>> +       }
>>> +   }
>>> +
> 
>> It seems like we miss OES_texture_float as well .. with the above change
>> (separation of half float formats from full float ones) we could have a
>> OES_texture_float || ARB_texture_float check for the rest?
> 
> 
> Yeah.. makes sense.
> 
> 
>>>       if (ctx->Extensions.ARB_texture_float) {
>>>          switch (internalFormat) {
>>>          case GL_ALPHA16F_ARB:
>>>
> 
> 
> What about something like this:
> 
> -----8<-------
> 
> iff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> index cba5e670db..f6c252cdf0 100644
> --- a/src/mesa/main/glformats.c
> +++ b/src/mesa/main/glformats.c
> @@ -2424,7 +2424,27 @@ _mesa_base_tex_format(const struct gl_context *ctx,
> GLint internalFormat)
>             return GL_YCBCR_MESA;
>       }
> 
> -   if (ctx->Extensions.ARB_texture_float) {
> +   if (ctx->Extensions.OES_texture_half_float) {
> +       switch (internalFormat) {
> +       case GL_ALPHA16F_ARB:
> +          return GL_ALPHA;
> +       case GL_RGBA16F_ARB:
> +          return GL_RGBA;
> +       case GL_RGB16F_ARB:
> +          return GL_RGB;
> +       case GL_INTENSITY16F_ARB:
> +          return GL_INTENSITY;
> +       case GL_LUMINANCE16F_ARB:
> +          return GL_LUMINANCE;
> +       case GL_LUMINANCE_ALPHA16F_ARB:
> +          return GL_LUMINANCE_ALPHA;
> +       default:
> +          ; /* fallthrough */
> +       }
> +   }
> +
> +   if (ctx->Extensions.ARB_texture_float ||
> +       ctx->Extensions.OES_texture_float) {
>          switch (internalFormat) {
>          case GL_ALPHA16F_ARB:
>          case GL_ALPHA32F_ARB:
> 
> -----8<-------


Right this is what I was after, I believe you should be able to also 
remove all the 16F formats from the list below since they are already 
checked above. So first check 16F ones and then 32F ones separately.

// Tapani