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

[Stéphane Marchesin]

2011/6/17 Brian Paul <brianp at vmware.com>:
> 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.
>

Well, if we do that, we have RGBA8 taking precedence in the BGRA case
also, which now makes BGRA transfers slow because there is format
conversion going on...
I guess what I want is the following: implement RGBA8 and BGRA8 in the
driver, and when apps request a texture we try to use a format that
require no conversion.

> 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.
>

Hmm you're right. So the reason is that I implemented native RGBA
support in i915g (it currently only has BGRA), but it doesn't get used
because BGRA gets used instead in all cases. I guess right now I'd
like to see the BGRA driver format used when the user gives us
RGBA/BGRA and the RGBA format used when the user gives us RGBA/RGBA
(that would avoid conversion in both cases). Should I just add a
heuristic for that or do we want to be more generic? Can you think of
other cases we want to handle? I just want to avoid the conversion
overhead for those basic formats.

Stéphane