[Mesa-dev] [PATCH 2/3] mesa: restrict formats being supported by target type for formatquery

[Alejandro Piñeiro]

On 27/01/18 01:59, sroland at vmware.com wrote:
> From: Roland Scheidegger <sroland at vmware.com>
>
> The code just considered all formats as being supported if they were either
> a valid fbo or texture format.
> This was quite awkward since then the query would return "supported" for
> e.g. GL_RGB9E5 or compressed formats and target RENDERBUFFER (albeit the driver
> could still refuse it in theory). However, when then querying for instance the
> internalformat sizes, it would just return 0 (due to the checks being more
> strict there).
> It was also a problem for texture buffer targets, which have a more restricted
> list of formats which are allowed (and again, it would return supported but
> then querying sizes would return 0).
> So only take validation of formats into account which make sense for a given
> target.
> Can also toss out some special checks for rgb9e5 later, since we'd never get
> there if it wasn't supported in the first place.

The texture buffer/renderbuffer thing was raised some months ago. Good
catch with the additional spec paragraph, that clarifies several cases.

The patch:
Reviewed-by: Alejandro Piñeiro <apinheiro at igalia.com>

> ---
>  src/mesa/main/formatquery.c | 31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/src/mesa/main/formatquery.c b/src/mesa/main/formatquery.c
> index e0062a64d2..2214f97e67 100644
> --- a/src/mesa/main/formatquery.c
> +++ b/src/mesa/main/formatquery.c
> @@ -558,15 +558,29 @@ _is_internalformat_supported(struct gl_context *ctx, GLenum target,
>      *         implementation accepts it for any texture specification commands, and
>      *         - unsized or base internal format, if the implementation accepts
>      *         it for texture or image specification.
> +    *
> +    * But also:
> +    * "If the particualar <target> and <internalformat> combination do not make
> +    * sense, or if a particular type of <target> is not supported by the
> +    * implementation the "unsupported" answer should be given. This is not an
> +    * error.
>      */
>     GLint buffer[1];
>  
> -   /* At this point an internalformat is valid if it is valid as a texture or
> -    * as a renderbuffer format. The checks are different because those methods
> -    * return different values when passing non supported internalformats */
> -   if (_mesa_base_tex_format(ctx, internalformat) < 0 &&
> -       _mesa_base_fbo_format(ctx, internalformat) == 0)
> -      return false;
> +   if (target == GL_RENDERBUFFER) {
> +      if (_mesa_base_fbo_format(ctx, internalformat) == 0) {
> +         return false;
> +      }
> +   } else if (target == GL_TEXTURE_BUFFER) {
> +      if (_mesa_validate_texbuffer_format(ctx, internalformat) ==
> +          MESA_FORMAT_NONE) {
> +         return false;
> +      }
> +   } else {
> +      if (_mesa_base_tex_format(ctx, internalformat) < 0) {
> +         return false;
> +      }
> +   }
>  
>     /* Let the driver have the final word */
>     ctx->Driver.QueryInternalFormat(ctx, target, internalformat,
> @@ -969,10 +983,7 @@ _mesa_GetInternalformativ(GLenum target, GLenum internalformat, GLenum pname,
>         * and glGetRenderbufferParameteriv functions.
>         */
>        if (pname == GL_INTERNALFORMAT_SHARED_SIZE) {
> -         if (_mesa_has_EXT_texture_shared_exponent(ctx) &&
> -             target != GL_TEXTURE_BUFFER &&
> -             target != GL_RENDERBUFFER &&
> -             texformat == MESA_FORMAT_R9G9B9E5_FLOAT) {
> +         if (texformat == MESA_FORMAT_R9G9B9E5_FLOAT) {
>              buffer[0] = 5;
>           }
>           goto end;