[Mesa-dev] [PATCH 10/24] swrast: Make the packed depth/stencil read fastpath use MapRenderbuffer.

[Brian Paul]

On Fri, Oct 28, 2011 at 1:50 PM, Eric Anholt <eric at anholt.net> wrote:
> This also makes it handle 24/8 vs 8/24, fixing piglit
> depthstencil-default_fb-readpixels-24_8 on i965.  While here, avoid
> incorrectly fast-pathing if packing->SwapBytes is set.
> ---
>  src/mesa/swrast/s_readpix.c |   94 +++++++++++++++++++++++++++++-------------
>  1 files changed, 65 insertions(+), 29 deletions(-)
>
> diff --git a/src/mesa/swrast/s_readpix.c b/src/mesa/swrast/s_readpix.c
> index 4ecb5b8..c0bb399 100644
> --- a/src/mesa/swrast/s_readpix.c
> +++ b/src/mesa/swrast/s_readpix.c
> @@ -392,6 +392,65 @@ read_rgba_pixels( struct gl_context *ctx,
>    }
>  }
>
> +/**
> + * For a packed depth/stencil buffer being read as depth/stencil, memcpy the
> + * data (possibly swapping 8/24 vs 24/8 as we go).
> + */
> +static GLboolean
> +fast_read_depth_stencil_pixels(struct gl_context *ctx,
> +                              GLint x, GLint y,
> +                              GLsizei width, GLsizei height,
> +                              GLenum type, GLvoid *pixels,
> +                              const struct gl_pixelstore_attrib *packing)
> +{
> +   struct gl_framebuffer *fb = ctx->ReadBuffer;
> +   struct gl_renderbuffer *rb = fb->Attachment[BUFFER_DEPTH].Renderbuffer;
> +   struct gl_renderbuffer *stencilRb = fb->Attachment[BUFFER_DEPTH].Renderbuffer;

I think that's a typo:  s/BUFFER_DEPTH/BUFFER_STENCIL/ there.


> +   GLubyte *dst, *map;
> +   int stride, dstStride, i;
> +
> +   if (rb != stencilRb)
> +      return GL_FALSE;
> +
> +   if (type != GL_UNSIGNED_INT_24_8)
> +      return GL_FALSE;
> +
> +   if (rb->Format != MESA_FORMAT_Z24_S8 &&
> +       rb->Format != MESA_FORMAT_S8_Z24)
> +      return GL_FALSE;
> +
> +   ctx->Driver.MapRenderbuffer(ctx, rb, x, y, width, height, GL_MAP_READ_BIT,
> +                              &map, &stride);
> +
> +   dstStride = _mesa_image_row_stride(packing, width,
> +                                     GL_DEPTH_STENCIL_EXT, type);
> +   dst = (GLubyte *) _mesa_image_address2d(packing, pixels,
> +                                          width, height,
> +                                          GL_DEPTH_STENCIL_EXT,
> +                                          type, 0, 0);
> +
> +   for (i = 0; i < height; i++) {
> +      memcpy(dst, map, width * 4);
> +
> +      if (rb->Format == MESA_FORMAT_S8_Z24) {
> +        int j;
> +        uint32_t *data = (uint32_t *)dst;
> +
> +        for (j = 0; j < width; j++) {
> +           uint32_t val = *data;
> +           *(data++) = (val >> 24) | (val << 8);

I think using array indexes is cleaner here:

   uint32_t val = data[j]
   data[j] = (val >> 24) | (val << 8;


Looks good otherwise.

-Brian