[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 02:12 PM, Tapani Pälli wrote:
> 
> 
> 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.
> 

So for this separation to happen we would have ...

for 16F ones:

if (ctx->Extensions.ARB_texture_float || 
ctx->Extensions.OES_texture_half_float)

and another one for 32F ones:

if (ctx->Extensions.ARB_texture_float ||
ctx->Extensions.OES_texture_float)


// Tapani