[Mesa-dev] [PATCH 2/2] st/mesa: don't choose DXT formats if we can't do DXT compression

Jose Fonseca jfonseca at vmware.com
Fri Feb 1 07:13:29 PST 2013



----- Original Message -----
> If we call glTexImage2D() with a generic compression format (e.g.
> intFormat=GL_COMPRESSED_RGBA) we can't choose a DXT format if we
> don't have the external DXT compression library.
> 
> We weren't actually enforcing this before since the
> pipe_screen::is_format_supported(DXT) query has no dependency on
> the DXT compression library.
> 
> Now if we're given a generic compressed format and we can't do DXT
> compression we'll fall back to a non-compressed format.
> 
> Note: This is a candidate for the stable branches.
> ---
>  src/mesa/state_tracker/st_cb_drawpixels.c |    6 ++-
>  src/mesa/state_tracker/st_cb_texture.c    |    2 +-
>  src/mesa/state_tracker/st_format.c        |   47
>  ++++++++++++++++++++++++-----
>  src/mesa/state_tracker/st_format.h        |    2 +-
>  src/mesa/state_tracker/st_texture.c       |    3 +-
>  5 files changed, 47 insertions(+), 13 deletions(-)
> 
> diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c
> b/src/mesa/state_tracker/st_cb_drawpixels.c
> index c944b81..c3c5326 100644
> --- a/src/mesa/state_tracker/st_cb_drawpixels.c
> +++ b/src/mesa/state_tracker/st_cb_drawpixels.c
> @@ -1499,14 +1499,16 @@ st_CopyPixels(struct gl_context *ctx, GLint
> srcx, GLint srcy,
>        if (type == GL_DEPTH) {
>           texFormat = st_choose_format(screen, GL_DEPTH_COMPONENT,
>                                        GL_NONE, GL_NONE,
>                                        st->internal_target,
> -				      sample_count, PIPE_BIND_DEPTH_STENCIL);
> +                                      sample_count,
> PIPE_BIND_DEPTH_STENCIL,
> +                                      FALSE);
>           assert(texFormat != PIPE_FORMAT_NONE);
>        }
>        else {
>           /* default color format */
>           texFormat = st_choose_format(screen, GL_RGBA,
>                                        GL_NONE, GL_NONE,
>                                        st->internal_target,
> -                                      sample_count,
> PIPE_BIND_SAMPLER_VIEW);
> +                                      sample_count,
> PIPE_BIND_SAMPLER_VIEW,
> +                                      FALSE);
>           assert(texFormat != PIPE_FORMAT_NONE);
>        }
>     }
> diff --git a/src/mesa/state_tracker/st_cb_texture.c
> b/src/mesa/state_tracker/st_cb_texture.c
> index 3cea2df..80a440d 100644
> --- a/src/mesa/state_tracker/st_cb_texture.c
> +++ b/src/mesa/state_tracker/st_cb_texture.c
> @@ -597,7 +597,7 @@ decompress_with_blit(struct gl_context * ctx,
>  
>     /* Find the best match for the format+type combo. */
>     pipe_format = st_choose_format(pipe->screen, GL_RGBA8, format,
>     type,
> -                                  pipe_target, 0, bind);
> +                                  pipe_target, 0, bind, FALSE);
>     if (pipe_format == PIPE_FORMAT_NONE) {
>        /* unable to get an rgba format!?! */
>        _mesa_problem(ctx, "%s: cannot find a supported format",
>        __func__);
> diff --git a/src/mesa/state_tracker/st_format.c
> b/src/mesa/state_tracker/st_format.c
> index 7ef0639..c3b286a 100644
> --- a/src/mesa/state_tracker/st_format.c
> +++ b/src/mesa/state_tracker/st_format.c
> @@ -1397,19 +1397,48 @@ static const struct format_mapping
> format_map[] = {
>  
>  
>  /**
> + * Is the given pipe format a DXT format?
> + */
> +static boolean
> +is_dxt_format(enum pipe_format format)
> +{
> +   switch (format) {
> +   case PIPE_FORMAT_DXT1_RGB:
> +   case PIPE_FORMAT_DXT1_RGBA:
> +   case PIPE_FORMAT_DXT3_RGBA:
> +   case PIPE_FORMAT_DXT5_RGBA:
> +   case PIPE_FORMAT_DXT1_SRGB:
> +   case PIPE_FORMAT_DXT1_SRGBA:
> +   case PIPE_FORMAT_DXT3_SRGBA:
> +   case PIPE_FORMAT_DXT5_SRGBA:
> +      return TRUE;
> +   default:
> +      return FALSE;
> +   }
> +}

I think you could use u_format.h's util_format_is_s3tc here.

> +
> +/**
>   * Return first supported format from the given list.
> + * \param allow_dxt  indicates whether it's OK to return a DXT
> format.
>   */
>  static enum pipe_format
>  find_supported_format(struct pipe_screen *screen,
>                        const enum pipe_format formats[],
>                        enum pipe_texture_target target,
>                        unsigned sample_count,
> -                      unsigned tex_usage)
> +                      unsigned tex_usage,
> +                      boolean allow_dxt)
>  {
>     uint i;
>     for (i = 0; formats[i]; i++) {
>        if (screen->is_format_supported(screen, formats[i], target,
>                                        sample_count, tex_usage)) {
> +         if (!allow_dxt && is_dxt_format(formats[i])) {
> +            /* we can't return a dxt format, continue searching */
> +            continue;
> +         }
> +
>           return formats[i];
>        }
>     }
> @@ -1519,7 +1548,7 @@ enum pipe_format
>  st_choose_format(struct pipe_screen *screen, GLenum internalFormat,
>                   GLenum format, GLenum type,
>                   enum pipe_texture_target target, unsigned
>                   sample_count,
> -                 unsigned bindings)
> +                 unsigned bindings, boolean allow_dxt)

It looks like parameter is serving two duties:

- some cases it is set to FALSE because we don't have the DXTn compression library available

- some cases it is set to FALSE because we don't want compressed textures, but truth is that setting FALSE will only black lists DXT and happily return non-DXT compressed libraries (KTX, etc).

If so, then I'd suggest rename allow_dxt to allow_compressed, and check ctx->Mesa_DXTn when is_dxt_format() is true.

Otherwise the series looks good.

Jose

>  {
>     GET_CURRENT_CONTEXT(ctx); /* XXX this should be a function
>     parameter */
>     int i, j;
> @@ -1547,7 +1576,8 @@ st_choose_format(struct pipe_screen *screen,
> GLenum internalFormat,
>               * which is supported by the driver.
>               */
>              return find_supported_format(screen,
>              mapping->pipeFormats,
> -                                         target, sample_count,
> bindings);
> +                                         target, sample_count,
> bindings,
> +                                         allow_dxt);
>           }
>        }
>     }
> @@ -1569,8 +1599,8 @@ st_choose_renderbuffer_format(struct
> pipe_screen *screen,
>        usage = PIPE_BIND_DEPTH_STENCIL;
>     else
>        usage = PIPE_BIND_RENDER_TARGET;
> -   return st_choose_format(screen, internalFormat, GL_NONE, GL_NONE,
> PIPE_TEXTURE_2D,
> -                           sample_count, usage);
> +   return st_choose_format(screen, internalFormat, GL_NONE, GL_NONE,
> +                           PIPE_TEXTURE_2D, sample_count, usage,
> FALSE);
>  }
>  
>  
> @@ -1597,12 +1627,13 @@ st_ChooseTextureFormat_renderable(struct
> gl_context *ctx, GLint internalFormat,
>     }
>  
>     pFormat = st_choose_format(screen, internalFormat, format, type,
> -                              PIPE_TEXTURE_2D, 0, bindings);
> +                              PIPE_TEXTURE_2D, 0, bindings,
> ctx->Mesa_DXTn);
>  
>     if (pFormat == PIPE_FORMAT_NONE) {
>        /* try choosing format again, this time without render target
>        bindings */
>        pFormat = st_choose_format(screen, internalFormat, format,
>        type,
> -                                 PIPE_TEXTURE_2D, 0,
> PIPE_BIND_SAMPLER_VIEW);
> +                                 PIPE_TEXTURE_2D, 0,
> PIPE_BIND_SAMPLER_VIEW,
> +                                 ctx->Mesa_DXTn);
>     }
>  
>     if (pFormat == PIPE_FORMAT_NONE) {
> @@ -1661,7 +1692,7 @@ st_QuerySamplesForFormat(struct gl_context
> *ctx, GLenum internalFormat,
>     /* Set sample counts in descending order. */
>     for (i = 16; i > 1; i--) {
>        format = st_choose_format(screen, internalFormat, GL_NONE,
>        GL_NONE,
> -                                PIPE_TEXTURE_2D, i, bind);
> +                                PIPE_TEXTURE_2D, i, bind, FALSE);
>  
>        if (format != PIPE_FORMAT_NONE) {
>           samples[num_sample_counts++] = i;
> diff --git a/src/mesa/state_tracker/st_format.h
> b/src/mesa/state_tracker/st_format.h
> index cb6e5bc..eac3cfb 100644
> --- a/src/mesa/state_tracker/st_format.h
> +++ b/src/mesa/state_tracker/st_format.h
> @@ -51,7 +51,7 @@ extern enum pipe_format
>  st_choose_format(struct pipe_screen *screen, GLenum internalFormat,
>                   GLenum format, GLenum type,
>                   enum pipe_texture_target target, unsigned
>                   sample_count,
> -                 unsigned bindings);
> +                 unsigned bindings, boolean allow_dxt);
>  
>  extern enum pipe_format
>  st_choose_renderbuffer_format(struct pipe_screen *screen,
> diff --git a/src/mesa/state_tracker/st_texture.c
> b/src/mesa/state_tracker/st_texture.c
> index ee4d762..584eaa9 100644
> --- a/src/mesa/state_tracker/st_texture.c
> +++ b/src/mesa/state_tracker/st_texture.c
> @@ -398,7 +398,8 @@ st_create_color_map_texture(struct gl_context
> *ctx)
>  
>     /* find an RGBA texture format */
>     format = st_choose_format(pipe->screen, GL_RGBA, GL_NONE,
>     GL_NONE,
> -                             PIPE_TEXTURE_2D, 0,
> PIPE_BIND_SAMPLER_VIEW);
> +                             PIPE_TEXTURE_2D, 0,
> PIPE_BIND_SAMPLER_VIEW,
> +                             FALSE);
>  
>     /* create texture for color map/table */
>     pt = st_texture_create(st, PIPE_TEXTURE_2D, format, 0,
> --
> 1.7.3.4
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 


More information about the mesa-dev mailing list