[Mesa-dev] [PATCH] st/mesa: add support for internalformat query2.

Alejandro Piñeiro apinheiro at igalia.com
Fri Mar 4 09:49:23 UTC 2016


Although I don't too much experience on the state-tracker section of
mesa codebase, the patch looks good to me. Just a comment below

On 04/03/16 03:36, Dave Airlie wrote:
> From: Dave Airlie <airlied at redhat.com>
>
> Add code to handle GL_INTERNALFORMAT_PREFERRED.
> Add code to deal with GL_RENDERBUFFER being passes into ChooseTextureFormat.
>
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>  docs/GL3.txt                           |  2 +-
>  src/mesa/state_tracker/st_extensions.c |  1 +
>  src/mesa/state_tracker/st_format.c     | 53 +++++++++++++++++++++++++++-------
>  3 files changed, 45 insertions(+), 11 deletions(-)
>
> diff --git a/docs/GL3.txt b/docs/GL3.txt
> index 02dc6ea..90387ce 100644
> --- a/docs/GL3.txt
> +++ b/docs/GL3.txt
> @@ -158,7 +158,7 @@ GL 4.3, GLSL 4.30:
>    GL_ARB_explicit_uniform_location                     DONE (all drivers that support GLSL)
>    GL_ARB_fragment_layer_viewport                       DONE (i965, nv50, nvc0, r600, radeonsi, llvmpipe)
>    GL_ARB_framebuffer_no_attachments                    DONE (i965)
> -  GL_ARB_internalformat_query2                         DONE (i965)
> +  GL_ARB_internalformat_query2                         DONE (all drivers)
>    GL_ARB_invalidate_subdata                            DONE (all drivers)
>    GL_ARB_multi_draw_indirect                           DONE (i965, nvc0, r600, radeonsi, llvmpipe, softpipe)
>    GL_ARB_program_interface_query                       DONE (all drivers)
> diff --git a/src/mesa/state_tracker/st_extensions.c b/src/mesa/state_tracker/st_extensions.c
> index 24c6444..43c19c3 100644
> --- a/src/mesa/state_tracker/st_extensions.c
> +++ b/src/mesa/state_tracker/st_extensions.c
> @@ -742,6 +742,7 @@ void st_init_extensions(struct pipe_screen *screen,
>     extensions->ARB_fragment_shader = GL_TRUE;
>     extensions->ARB_half_float_vertex = GL_TRUE;
>     extensions->ARB_internalformat_query = GL_TRUE;
> +   extensions->ARB_internalformat_query2 = GL_TRUE;
>     extensions->ARB_map_buffer_range = GL_TRUE;
>     extensions->ARB_texture_border_clamp = GL_TRUE; /* XXX temp */
>     extensions->ARB_texture_cube_map = GL_TRUE;
> diff --git a/src/mesa/state_tracker/st_format.c b/src/mesa/state_tracker/st_format.c
> index 5392c23..9b71c18 100644
> --- a/src/mesa/state_tracker/st_format.c
> +++ b/src/mesa/state_tracker/st_format.c
> @@ -2216,7 +2216,14 @@ st_ChooseTextureFormat(struct gl_context *ctx, GLenum target,
>     enum pipe_format pFormat;
>     mesa_format mFormat;
>     unsigned bindings;
> -   enum pipe_texture_target pTarget = gl_target_to_pipe(target);
> +   bool is_renderbuffer = false;
> +   enum pipe_texture_target pTarget;
> +
> +   if (target == GL_RENDERBUFFER) {
> +      pTarget = PIPE_TEXTURE_2D;
> +      is_renderbuffer = true;
> +   } else
> +      pTarget = gl_target_to_pipe(target);

On most places with a multiline if path, else path also uses brackets,
even on one-liners. Having said so, there are also a lot of places that
don't do that, so probably not really important.

>     if (target == GL_TEXTURE_1D || target == GL_TEXTURE_1D_ARRAY) {
>        /* We don't do compression for these texture targets because of
> @@ -2234,7 +2241,7 @@ st_ChooseTextureFormat(struct gl_context *ctx, GLenum target,
>     bindings = PIPE_BIND_SAMPLER_VIEW;
>     if (_mesa_is_depth_or_stencil_format(internalFormat))
>        bindings |= PIPE_BIND_DEPTH_STENCIL;
> -   else if (internalFormat == 3 || internalFormat == 4 ||
> +   else if (is_renderbuffer || internalFormat == 3 || internalFormat == 4 ||
>              internalFormat == GL_RGB || internalFormat == GL_RGBA ||
>              internalFormat == GL_RGB8 || internalFormat == GL_RGBA8 ||
>              internalFormat == GL_BGRA ||
> @@ -2267,19 +2274,21 @@ st_ChooseTextureFormat(struct gl_context *ctx, GLenum target,
>           if (pFormat != PIPE_FORMAT_NONE)
>              return st_pipe_format_to_mesa_format(pFormat);
>  
> -         /* try choosing format again, this time without render target bindings */
> -         pFormat = st_choose_matching_format(st, PIPE_BIND_SAMPLER_VIEW,
> -                                             format, type,
> -                                             ctx->Unpack.SwapBytes);
> -         if (pFormat != PIPE_FORMAT_NONE)
> -            return st_pipe_format_to_mesa_format(pFormat);
> +         if (!is_renderbuffer) {
> +            /* try choosing format again, this time without render target bindings */
> +            pFormat = st_choose_matching_format(st, PIPE_BIND_SAMPLER_VIEW,
> +                                                format, type,
> +                                                ctx->Unpack.SwapBytes);
> +            if (pFormat != PIPE_FORMAT_NONE)
> +               return st_pipe_format_to_mesa_format(pFormat);
> +         }
>        }
>     }
>  
>     pFormat = st_choose_format(st, internalFormat, format, type,
>                                pTarget, 0, bindings, ctx->Mesa_DXTn);
>  
> -   if (pFormat == PIPE_FORMAT_NONE) {
> +   if (pFormat == PIPE_FORMAT_NONE && !is_renderbuffer) {
>        /* try choosing format again, this time without render target bindings */
>        pFormat = st_choose_format(st, internalFormat, format, type,
>                                   pTarget, 0, PIPE_BIND_SAMPLER_VIEW,
> @@ -2357,6 +2366,7 @@ void
>  st_QueryInternalFormat(struct gl_context *ctx, GLenum target,
>                         GLenum internalFormat, GLenum pname, GLint *params)
>  {
> +   struct st_context *st = st_context(ctx);
>     /* The API entry-point gives us a temporary params buffer that is non-NULL
>      * and guaranteed to have at least 16 elements.
>      */
> @@ -2374,7 +2384,30 @@ st_QueryInternalFormat(struct gl_context *ctx, GLenum target,
>        params[0] = (GLint) num_samples;
>        break;
>     }
> -
> +   case GL_INTERNALFORMAT_PREFERRED: {
> +      params[0] = GL_NONE;
> +
> +      /* We need to resolve an internal format that is compatible with
> +       * the passed internal format, and optimal to the driver. By now,
> +       * we just validate that the passed internal format is supported by
> +       * the driver, and if so return the same internal format, otherwise
> +       * return GL_NONE.
> +       */
> +      uint usage;
> +      if (_mesa_is_depth_or_stencil_format(internalFormat))
> +         usage = PIPE_BIND_DEPTH_STENCIL;
> +      else
> +         usage = PIPE_BIND_RENDER_TARGET;
> +      enum pipe_format pformat = st_choose_format(st,
> +                                                  internalFormat,
> +                                                  GL_NONE,
> +                                                  GL_NONE,
> +                                                  PIPE_TEXTURE_2D, 1,
> +                                                  usage, FALSE);
> +      if (pformat)
> +         params[0] = internalFormat;
> +      break;
> +   }
>     default:
>        /* For the rest of the pnames, we call back the Mesa's default
>         * function for drivers that don't implement ARB_internalformat_query2.

With or without that change:

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

BR


More information about the mesa-dev mailing list