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

[Marek Olšák]

On Tue, Oct 24, 2017 at 12:09 PM, Alejandro Piñeiro
<apinheiro at igalia.com> wrote:
> 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.

Are the piglit tests wrong then?

Marek