[Mesa-dev] [PATCH] st/mesa: try the app's texture format first before using the internal format.

[Brian Paul]

On 06/17/2011 12:34 PM, Stéphane Marchesin wrote:
> If we can find it, that means we don't need to do texture format conversion
> and therefore we get fast texture uploads for natively supported formats.
> ---
>   src/mesa/state_tracker/st_format.c |   25 +++++++++++++++++--------
>   1 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_format.c b/src/mesa/state_tracker/st_format.c
> index 3583571..e39d835 100644
> --- a/src/mesa/state_tracker/st_format.c
> +++ b/src/mesa/state_tracker/st_format.c
> @@ -615,16 +615,16 @@ static struct format_mapping format_map[] = {
>         { PIPE_FORMAT_B10G10R10A2_UNORM, DEFAULT_RGBA_FORMATS }
>      },
>      {
> -      { 4, GL_RGBA, GL_RGBA8, 0 },
> -      { DEFAULT_RGBA_FORMATS, 0 }
> -   },
> -   {
>         { GL_BGRA, 0 },
>         { PIPE_FORMAT_B8G8R8A8_UNORM, DEFAULT_RGBA_FORMATS }
>      },
>      {
> +      { 4, GL_RGBA, GL_RGBA8, 0 },
> +      { PIPE_FORMAT_R8G8B8A8_UNORM, DEFAULT_RGBA_FORMATS }
> +   },
> +   {
>         { 3, GL_RGB, GL_RGB8, 0 },
> -      { DEFAULT_RGB_FORMATS, 0 }
> +      { PIPE_FORMAT_R8G8B8X8_UNORM, DEFAULT_RGB_FORMATS }
>      },

We should just add PIPE_FORMAT_R8G8B8A8_UNORM to DEFAULT_RGBA_FORMATS.

There's no mesa format that matches PIPE_FORMAT_R8G8B8X8_UNORM at this 
time.


>      {
>         { GL_RGB12, GL_RGB16, GL_RGBA12, GL_RGBA16, 0 },
> @@ -1108,7 +1108,7 @@ static struct format_mapping format_map[] = {
>    * Return first supported format from the given list.
>    */
>   static enum pipe_format
> -find_supported_format(struct pipe_screen *screen,
> +find_supported_format(struct pipe_screen *screen,
>                         const enum pipe_format formats[],
>                         enum pipe_texture_target target,
>                         unsigned sample_count,
> @@ -1210,14 +1210,23 @@ st_ChooseTextureFormat_renderable(struct gl_context *ctx, GLint internalFormat,
>         if (_mesa_is_depth_format(internalFormat) ||
>   	  _mesa_is_depth_or_stencil_format(internalFormat))
>   	 bindings |= PIPE_BIND_DEPTH_STENCIL;
> -      else
> +      else
>   	 bindings |= PIPE_BIND_RENDER_TARGET;
>      }

I'll take care of the trailing whitespace in a separate commit.


> -   pFormat = st_choose_format(screen, internalFormat,
> +   /* First try a format which matches the format provided by the app
> +    * This heuristic avoids potentially costly texture format conversions
> +    * and gets us much faster texture transfers. */
> +   pFormat = st_choose_format(screen, format,
>                                 PIPE_TEXTURE_2D, 0, bindings);
>
>      if (pFormat == PIPE_FORMAT_NONE) {
> +      /* Now try the internal format */
> +      pFormat = st_choose_format(screen, internalFormat,
> +                                 PIPE_TEXTURE_2D, 0, bindings);
> +   }
> +
> +   if (pFormat == PIPE_FORMAT_NONE) {
>         /* try choosing format again, this time without render target bindings */
>         pFormat = st_choose_format(screen, internalFormat,
>                                    PIPE_TEXTURE_2D, 0, PIPE_BIND_SAMPLER_VIEW);

I don't think this is right.  The internalFormat is more important the 
format parameter.

Suppose the user called glTexImage(internalFormat=GL_RGBA32F_ARB, 
format=GL_RGBA, type=GL_FLOAT).  We can't choose the hw format based 
on format=GL_RGBA since we won't get the float format the user wants.

-Brian