[Mesa-dev] [PATCH 4/4] st/mesa: optimize 4-component ubyte glDrawPixels

[Brian Paul]

On 10/16/2015 05:53 PM, Jose Fonseca wrote:
> On 15/10/15 20:01, Brian Paul wrote:
>> If we didn't find a gallium surface format that exactly matched the
>> glDrawPixels format/type combination, we used some other 32-bit packed
>> RGBA format and swizzled the whole image in the mesa texstore/format
>> code.
>>
>> That slow path can be avoided in some common cases by using the
>> pipe_samper_view's swizzle terms to do the swizzling at texture sampling
>> time instead.
>>
>> For now, only GL_RGBA/ubyte and GL_BGRA/ubyte combinations are supported.
>> In the future other formats and types like GL_UNSIGNED_INT_8_8_8_8 could
>> be added.
>> ---
>>   src/mesa/state_tracker/st_cb_drawpixels.c | 73
>> +++++++++++++++++++++++++++----
>>   1 file changed, 64 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c
>> b/src/mesa/state_tracker/st_cb_drawpixels.c
>> index 05f6e6b..a135761 100644
>> --- a/src/mesa/state_tracker/st_cb_drawpixels.c
>> +++ b/src/mesa/state_tracker/st_cb_drawpixels.c
>> @@ -395,15 +395,35 @@ make_texture(struct st_context *st,
>>          * Note that the image is actually going to be upside down in
>>          * the texture.  We deal with that with texcoords.
>>          */
>> -      success = _mesa_texstore(ctx, 2,           /* dims */
>> -                               baseInternalFormat, /*
>> baseInternalFormat */
>> -                               mformat,          /* mesa_format */
>> -                               transfer->stride, /* dstRowStride,
>> bytes */
>> -                               &dest,            /* destSlices */
>> -                               width, height, 1, /* size */
>> -                               format, type,     /* src format/type */
>> -                               pixels,           /* data source */
>> -                               unpack);
>> +      if ((format == GL_RGBA || format == GL_BGRA)
>> +          && type == GL_UNSIGNED_BYTE) {
>> +         /* Use a memcpy-based texstore to avoid software pixel
>> swizzling.
>> +          * We'll do the necessary swizzling with the
>> pipe_sampler_view to
>> +          * give much better performance.
>> +          * XXX in the future, expand this to accomodate more format and
>> +          * type combinations.
>> +          */
>> +         _mesa_memcpy_texture(ctx, 2,
>> +                              mformat,          /* mesa_format */
>> +                              transfer->stride, /* dstRowStride,
>> bytes */
>> +                              &dest,            /* destSlices */
>> +                              width, height, 1, /* size */
>> +                              format, type,     /* src format/type */
>> +                              pixels,           /* data source */
>> +                              unpack);
>> +         success = GL_TRUE;
>> +      }
>> +      else {
>> +         success = _mesa_texstore(ctx, 2,           /* dims */
>> +                                  baseInternalFormat, /*
>> baseInternalFormat */
>> +                                  mformat,          /* mesa_format */
>> +                                  transfer->stride, /* dstRowStride,
>> bytes */
>> +                                  &dest,            /* destSlices */
>> +                                  width, height, 1, /* size */
>> +                                  format, type,     /* src
>> format/type */
>> +                                  pixels,           /* data source */
>> +                                  unpack);
>> +      }
>>
>>         /* unmap */
>>         pipe_transfer_unmap(pipe, transfer);
>> @@ -958,6 +978,38 @@ clamp_size(struct pipe_context *pipe, GLsizei
>> *width, GLsizei *height,
>>
>>
>>   /**
>> + * Set the sampler view's swizzle terms.  This is used to handle RGBA
>> + * swizzling when the incoming image format isn't an exact match for
>> + * the actual texture format.  For example, if we have glDrawPixels(
>> + * GL_RGBA, GL_UNSIGNED_BYTE) and we chose the texture format
>> + * PIPE_FORMAT_B8G8R8A8 then we can do use the sampler view swizzle to
>> + * avoid swizzling all the pixels in software in the texstore code.
>> + */
>> +static void
>> +setup_sampler_swizzle(struct pipe_sampler_view *sv, GLenum format,
>> GLenum type)
>> +{
>> +   if ((format == GL_RGBA || format == GL_BGRA) && type ==
>> GL_UNSIGNED_BYTE) {
>> +      const struct util_format_description *desc =
>> +         util_format_description(sv->texture->format);
>> +      /* Every gallium driver supports at least one 32-bit packed
>> RGBA format.
>> +       * We must have chosen one for (GL_RGBA, GL_UNSIGNED_BYTE).
>> +       */
>> +      assert(desc->block.bits == 32);
>> +      /* use the format's swizzle to setup the sampler swizzle */
>> +      sv->swizzle_r = desc->swizzle[0];
>> +      sv->swizzle_g = desc->swizzle[1];
>> +      sv->swizzle_b = desc->swizzle[2];
>> +      sv->swizzle_a = desc->swizzle[3];
>
> I think it should be the other way around: the sampler view's swizzle
> should _undo_ the format swizzle, not apply it again.
>
> This indeed works for RGBA8_URNOM / BGRA8_UNORM, but by mere
> coincidence.  It will fail for something like ABGR8_UNORM.
>
> If you don't want to deal with the swizzle inversion now, it might be
> better to explicitly check that the texture->format is RGBA8_URNOM /
> BGRA8_UNORM

I'll rework it.  I think I just got lucky with my testing.

-Brian