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

[Jose Fonseca]

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

Jose

> +      if (format == GL_BGRA) {
> +         /* swap red/blue */
> +         sv->swizzle_r = desc->swizzle[2];
> +         sv->swizzle_b = desc->swizzle[0];
> +      }
> +   }
> +}
> +
> +
> +/**
>    * Called via ctx->Driver.DrawPixels()
>    */
>   static void
> @@ -1046,6 +1098,9 @@ st_DrawPixels(struct gl_context *ctx, GLint x, GLint y,
>         return;
>      }
>
> +   /* Set up the sampler view's swizzle */
> +   setup_sampler_swizzle(sv[0], format, type);
> +
>      /* Create a second sampler view to read stencil.  The stencil is
>       * written using the shader stencil export functionality.
>       */
>