[Mesa-dev] [PATCH 1/3] mesa: return 0 for GL_INTERNALFORMAT_SUPPORTED for unsupported TBO formats

[Alejandro Piñeiro]

On 24/10/17 11:53, Marek Olšák wrote:
> On Tue, Oct 24, 2017 at 11:47 AM, Alejandro Piñeiro
> <apinheiro at igalia.com> wrote:
>> On 21/10/17 14:54, Marek Olšák wrote:
>>> From: Marek Olšák <marek.olsak at amd.com>
>>>
>>> ---
>>>  src/mesa/main/formatquery.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/src/mesa/main/formatquery.c b/src/mesa/main/formatquery.c
>>> index 77c7faa..05b7810 100644
>>> --- a/src/mesa/main/formatquery.c
>>> +++ b/src/mesa/main/formatquery.c
>>> @@ -895,20 +895,25 @@ _mesa_GetInternalformativ(GLenum target, GLenum internalformat, GLenum pname,
>>>        if (pname == GL_NUM_SAMPLE_COUNTS && ctx->API == API_OPENGLES2 &&
>>>            ctx->Version == 30 && _mesa_is_enum_format_integer(internalformat)) {
>>>           goto end;
>>>        }
>>>
>>>        ctx->Driver.QueryInternalFormat(ctx, target, internalformat, pname,
>>>                                        buffer);
>>>        break;
>>>
>>>     case GL_INTERNALFORMAT_SUPPORTED:
>>> +      /* Reject invalid texture buffer formats. */
>>> +      if (target == GL_TEXTURE_BUFFER &&
>>> +          _mesa_validate_texbuffer_format(ctx, internalformat) == MESA_FORMAT_NONE)
>>> +         break;
>>> +
>> _mesa_validate_texbuffer_format with that internalformat if target is
>> GL_TEXTURE_BUFFER is already called on _is_resource_supported, on line
>> 864. Why it is needed a second call here?
>>
> See the beginning of _is_resource_supported.

Ah true. internalformat_query2, that delightful spec. I'm starting to
remember. So from the spec:

"

    - INTERNALFORMAT_SUPPORTED: If <internalformat> is an internal format
      that is supported by the implementation in at least some subset of 
      possible operations, TRUE is written to <params>.  If <internalformat>
      if not a valid token for any internal format usage, FALSE is returned."

This pname is asking <internalformat> is supported somehow, on at least
one operation. And note that it says "format", and not "resource" that
in the wording of this spec for the pair target/internalformat.

Reading it literally, that means that this pname query is independent of
the target.

So, under that interpretation of your spec, your patch is wrong, as you
are checking against a specific target, but it should return TRUE as far
as it is supported for one case.

BR