[Mesa-dev] [PATCH] mesa: fix GL_{COLOR, DEPTH, STENCIL}_COMPONENTS queries for TBOs

[Alejandro Piñeiro]

On 24/10/17 03:28, Ilia Mirkin wrote:
> On Mon, Oct 23, 2017 at 9:16 PM, Marek Olšák <maraeo at gmail.com> wrote:
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> ---
>>  src/mesa/main/formatquery.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/src/mesa/main/formatquery.c b/src/mesa/main/formatquery.c
>> index 05b7810..9c53d7c 100644
>> --- a/src/mesa/main/formatquery.c
>> +++ b/src/mesa/main/formatquery.c
>> @@ -1094,43 +1094,54 @@ _mesa_GetInternalformativ(GLenum target, GLenum internalformat, GLenum pname,
>>     }
>>
>>     case GL_COLOR_COMPONENTS:
>>        /* The ARB_internalformat_query2 spec says:
>>         *
>>         *     "- COLOR_COMPONENTS: If the internal format contains any color
>>         *     components (R, G, B, or A), TRUE is returned in <params>.
>>         *     If the internal format is unsupported or contains no color
>>         *     components, FALSE is returned."
>>         */
>> +      if (target == GL_TEXTURE_BUFFER &&
>> +          _mesa_validate_texbuffer_format(ctx, internalformat) ==
>> +          MESA_FORMAT_NONE)
>> +         break;
> Because not all color formats are supported for TBO's, right?

But in the same case that with INTERNALFORMAT_SUPPORTED on the quote
spec, there is no reference to the format or the "resource", so the
literal reading of that paragraph is ignore the target, and only returns
if the format is supported in any combination. The method
_is_resource_supported is already doing that on purpose.

In any case, if we disagree with the interpretation of the spec, it
would be more simple to modify the method _is_resource_supported.

>
> Although _mesa_is_color_format returns true for unsupported formats as
> well... although that's probably a separate bug.
>
> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>

NAK unless someone convinces otherwise about the spec interpretation.

>
>> +
>>        if (_mesa_is_color_format(internalformat))
>>           buffer[0] = GL_TRUE;
>>        break;
>>
>>     case GL_DEPTH_COMPONENTS:
>>        /* The ARB_internalformat_query2 spec says:
>>         *
>>         *     "- DEPTH_COMPONENTS: If the internal format contains a depth
>>         *     component (D), TRUE is returned in <params>. If the internal format
>>         *     is unsupported or contains no depth component, FALSE is returned."
>>         */
>> +      if (target == GL_TEXTURE_BUFFER)
>> +         break;
>> +
>>        if (_mesa_is_depth_format(internalformat) ||
>>            _mesa_is_depthstencil_format(internalformat))
>>           buffer[0] = GL_TRUE;
>>        break;
>>
>>     case GL_STENCIL_COMPONENTS:
>>        /* The ARB_internalformat_query2 spec says:
>>         *
>>         *     "- STENCIL_COMPONENTS: If the internal format contains a stencil
>>         *     component (S), TRUE is returned in <params>. If the internal format
>>         *     is unsupported or contains no stencil component, FALSE is returned.
>>         */
>> +      if (target == GL_TEXTURE_BUFFER)
>> +         break;
>> +
>>        if (_mesa_is_stencil_format(internalformat) ||
>>            _mesa_is_depthstencil_format(internalformat))
>>           buffer[0] = GL_TRUE;
>>        break;
>>
>>     case GL_COLOR_RENDERABLE:
>>     case GL_DEPTH_RENDERABLE:
>>     case GL_STENCIL_RENDERABLE:
>>        if (!_is_renderable(ctx, internalformat))
>>           goto end;
>> --
>> 2.7.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev