[Mesa-dev] [PATCH 3/3] swrast: map/unmap attachments, not renderbuffers in glReadPixels

Brian Paul brianp at vmware.com
Sat Nov 5 12:15:41 PDT 2011


Most drivers have been creating fake/wrapper renderbuffers when a
texture image is attached to an FBO.  But that's not a requirement
of core Mesa.

So an FBO attachment that points into a texture may not have a
(fake) renderbuffer.  This caused the new glReadPixels code to fail
when reading from a buffer that's actually a texture.

Now, we check the attachment point to see if it's a real renderbuffer
or a texture image and call the corresponding MapRenderbuffer() or
MapTextureImage() function.

This fixes a bunch of piglit fbo failures when using the non-DRI
swrast driver.
---
 src/mesa/swrast/s_readpix.c |  242 ++++++++++++++++++++++++++++++-------------
 1 files changed, 171 insertions(+), 71 deletions(-)

diff --git a/src/mesa/swrast/s_readpix.c b/src/mesa/swrast/s_readpix.c
index 50422db..45d07d2 100644
--- a/src/mesa/swrast/s_readpix.c
+++ b/src/mesa/swrast/s_readpix.c
@@ -25,6 +25,7 @@
 
 #include "main/glheader.h"
 #include "main/colormac.h"
+#include "main/fbobject.h"
 #include "main/feedback.h"
 #include "main/formats.h"
 #include "main/format_unpack.h"
@@ -40,6 +41,95 @@
 #include "s_span.h"
 #include "s_stencil.h"
 
+
+/**
+ * Map a framebuffer attachment.  An attachment may either be a renderbuffer
+ * or a texture image.  Check the attachment type and map the corresponding
+ * image.
+ * Note:  This might be moved out of swrast someday.
+ */
+static void
+map_attachment(struct gl_context *ctx,
+               struct gl_renderbuffer_attachment *att,
+               GLuint x, GLuint y, GLuint w, GLuint h,
+               GLbitfield mode,
+               GLubyte **mapOut, GLint *rowStrideOut)
+{
+   if (att->Type == GL_TEXTURE) {
+      struct gl_texture_image *texImage = _mesa_get_attachment_teximage(att);
+      assert(texImage);
+      ctx->Driver.MapTextureImage(ctx, texImage,
+                                  att->Zoffset,
+                                  x, y, w, h, mode,
+                                  mapOut, rowStrideOut);
+   }
+   else if (att->Type == GL_RENDERBUFFER) {
+      assert(att->Renderbuffer);
+      ctx->Driver.MapRenderbuffer(ctx, att->Renderbuffer,
+                                  x, y, w, h, mode,
+                                  mapOut, rowStrideOut);
+   }
+   else {
+      *mapOut = NULL;
+      *rowStrideOut = 0;
+      _mesa_warning(NULL, "invalid attachment in map_attachment()");
+   }
+}
+
+
+/**
+ * Unmap a framebuffer attachment.
+ * Note:  This might be moved out of swrast someday.
+ */
+static void
+unmap_attachment(struct gl_context *ctx,
+                 struct gl_renderbuffer_attachment *att)
+{
+   if (att->Type == GL_TEXTURE) {
+      struct gl_texture_image *texImage = _mesa_get_attachment_teximage(att);
+      assert(texImage);
+      ctx->Driver.UnmapTextureImage(ctx, texImage, att->Zoffset);
+   }
+   else if (att->Type == GL_RENDERBUFFER) {
+      assert(att->Renderbuffer);
+      ctx->Driver.UnmapRenderbuffer(ctx, att->Renderbuffer);
+   }
+   else {
+      _mesa_warning(NULL, "invalid attachment in unmap_attachment()");
+   }
+}
+
+
+/**
+ * Get the format of a framebuffer attachment, which may either be a
+ * renderbuffer or texture image.  We shouldn't get here if there's no
+ * attachment at all.
+ * Note:  This might be moved out of swrast someday.
+ */
+static gl_format
+get_attachment_format(const struct gl_renderbuffer_attachment *att)
+{
+   if (att->Type == GL_TEXTURE) {
+      const struct gl_texture_image *texImage =
+         _mesa_get_attachment_teximage_const(att);
+      assert(texImage);
+      if (texImage)
+         return texImage->TexFormat;
+   }
+   else if (att->Type == GL_RENDERBUFFER) {
+      assert(att->Renderbuffer);
+      if (att->Renderbuffer)
+         return att->Renderbuffer->Format;
+   }
+   else {
+      _mesa_warning(NULL, "invalid attachment in get_attachment_format()");
+   }
+   /* we should normally never get here */
+   return MESA_FORMAT_NONE;
+}
+
+
+
 /**
  * Tries to implement glReadPixels() of GL_DEPTH_COMPONENT using memcpy of the
  * mapping.
@@ -52,7 +142,8 @@ fast_read_depth_pixels( struct gl_context *ctx,
 			const struct gl_pixelstore_attrib *packing )
 {
    struct gl_framebuffer *fb = ctx->ReadBuffer;
-   struct gl_renderbuffer *rb = fb->Attachment[BUFFER_DEPTH].Renderbuffer;
+   struct gl_renderbuffer_attachment *depthAtt = &fb->Attachment[BUFFER_DEPTH];
+   gl_format depthFormat = get_attachment_format(depthAtt);
    GLubyte *map, *dst;
    int stride, dstStride, j;
 
@@ -62,15 +153,15 @@ fast_read_depth_pixels( struct gl_context *ctx,
    if (packing->SwapBytes)
       return GL_FALSE;
 
-   if (_mesa_get_format_datatype(rb->Format) != GL_UNSIGNED_INT)
+   if (_mesa_get_format_datatype(depthFormat) != GL_UNSIGNED_INT)
       return GL_FALSE;
 
-   if (!((type == GL_UNSIGNED_SHORT && rb->Format == MESA_FORMAT_Z16) ||
+   if (!((type == GL_UNSIGNED_SHORT && depthFormat == MESA_FORMAT_Z16) ||
 	 type == GL_UNSIGNED_INT))
       return GL_FALSE;
 
-   ctx->Driver.MapRenderbuffer(ctx, rb, x, y, width, height, GL_MAP_READ_BIT,
-			       &map, &stride);
+   map_attachment(ctx, depthAtt, x, y, width, height, GL_MAP_READ_BIT,
+                  &map, &stride);
 
    dstStride = _mesa_image_row_stride(packing, width, GL_DEPTH_COMPONENT, type);
    dst = (GLubyte *) _mesa_image_address2d(packing, pixels, width, height,
@@ -78,16 +169,17 @@ fast_read_depth_pixels( struct gl_context *ctx,
 
    for (j = 0; j < height; j++) {
       if (type == GL_UNSIGNED_INT) {
-	 _mesa_unpack_uint_z_row(rb->Format, width, map, (GLuint *)dst);
+	 _mesa_unpack_uint_z_row(depthFormat, width, map, (GLuint *)dst);
       } else {
-	 ASSERT(type == GL_UNSIGNED_SHORT && rb->Format == MESA_FORMAT_Z16);
+	 ASSERT(type == GL_UNSIGNED_SHORT && depthFormat == MESA_FORMAT_Z16);
 	 memcpy(dst, map, width * 2);
       }
 
       map += stride;
       dst += dstStride;
    }
-   ctx->Driver.UnmapRenderbuffer(ctx, rb);
+
+   unmap_attachment(ctx, depthAtt);
 
    return GL_TRUE;
 }
@@ -103,19 +195,12 @@ read_depth_pixels( struct gl_context *ctx,
                    const struct gl_pixelstore_attrib *packing )
 {
    struct gl_framebuffer *fb = ctx->ReadBuffer;
-   struct gl_renderbuffer *rb = fb->Attachment[BUFFER_DEPTH].Renderbuffer;
+   struct gl_renderbuffer_attachment *depthAtt = &fb->Attachment[BUFFER_DEPTH];
+   gl_format depthFormat = get_attachment_format(depthAtt);
    GLint j;
    GLubyte *dst, *map;
    int dstStride, stride;
 
-   if (!rb)
-      return;
-
-   /* clipping should have been done already */
-   ASSERT(x >= 0);
-   ASSERT(y >= 0);
-   ASSERT(x + width <= (GLint) rb->Width);
-   ASSERT(y + height <= (GLint) rb->Height);
    /* width should never be > MAX_WIDTH since we did clipping earlier */
    ASSERT(width <= MAX_WIDTH);
 
@@ -126,20 +211,20 @@ read_depth_pixels( struct gl_context *ctx,
    dst = (GLubyte *) _mesa_image_address2d(packing, pixels, width, height,
 					   GL_DEPTH_COMPONENT, type, 0, 0);
 
-   ctx->Driver.MapRenderbuffer(ctx, rb, x, y, width, height, GL_MAP_READ_BIT,
-			       &map, &stride);
+   map_attachment(ctx, depthAtt, x, y, width, height, GL_MAP_READ_BIT,
+                  &map, &stride);
 
    /* General case (slower) */
    for (j = 0; j < height; j++, y++) {
       GLfloat depthValues[MAX_WIDTH];
-      _mesa_unpack_float_z_row(rb->Format, width, map, depthValues);
+      _mesa_unpack_float_z_row(depthFormat, width, map, depthValues);
       _mesa_pack_depth_span(ctx, width, dst, type, depthValues, packing);
 
       dst += dstStride;
       map += stride;
    }
 
-   ctx->Driver.UnmapRenderbuffer(ctx, rb);
+   unmap_attachment(ctx, depthAtt);
 }
 
 
@@ -154,26 +239,24 @@ read_stencil_pixels( struct gl_context *ctx,
                      const struct gl_pixelstore_attrib *packing )
 {
    struct gl_framebuffer *fb = ctx->ReadBuffer;
-   struct gl_renderbuffer *rb = fb->Attachment[BUFFER_STENCIL].Renderbuffer;
+   struct gl_renderbuffer_attachment *stencilAtt = &fb->Attachment[BUFFER_STENCIL];
+   gl_format stencilFormat = get_attachment_format(stencilAtt);
    GLint j;
    GLubyte *map;
    GLint stride;
 
-   if (!rb)
-      return;
-
    /* width should never be > MAX_WIDTH since we did clipping earlier */
    ASSERT(width <= MAX_WIDTH);
 
-   ctx->Driver.MapRenderbuffer(ctx, rb, x, y, width, height, GL_MAP_READ_BIT,
-			       &map, &stride);
+   map_attachment(ctx, stencilAtt, x, y, width, height, GL_MAP_READ_BIT,
+                  &map, &stride);
 
    /* process image row by row */
    for (j = 0; j < height; j++) {
       GLvoid *dest;
       GLstencil stencil[MAX_WIDTH];
 
-      _mesa_unpack_ubyte_stencil_row(rb->Format, width, map, stencil);
+      _mesa_unpack_ubyte_stencil_row(stencilFormat, width, map, stencil);
       dest = _mesa_image_address2d(packing, pixels, width, height,
                                    GL_STENCIL_INDEX, type, j, 0);
 
@@ -182,7 +265,7 @@ read_stencil_pixels( struct gl_context *ctx,
       map += stride;
    }
 
-   ctx->Driver.UnmapRenderbuffer(ctx, rb);
+   unmap_attachment(ctx, stencilAtt);
 }
 
 static GLboolean
@@ -194,11 +277,14 @@ fast_read_rgba_pixels_memcpy( struct gl_context *ctx,
 			      const struct gl_pixelstore_attrib *packing,
 			      GLbitfield transferOps )
 {
-   struct gl_renderbuffer *rb = ctx->ReadBuffer->_ColorReadBuffer;
+   struct gl_framebuffer *fb = ctx->ReadBuffer;
+   struct gl_renderbuffer_attachment *colorAtt =
+      &fb->Attachment[fb->_ColorReadBufferIndex];
+   gl_format colorFormat = get_attachment_format(colorAtt);
    GLubyte *dst, *map;
    int dstStride, stride, j, texelBytes;
 
-   if (!_mesa_format_matches_format_and_type(rb->Format, format, type))
+   if (!_mesa_format_matches_format_and_type(colorFormat, format, type))
       return GL_FALSE;
 
    /* check for things we can't handle here */
@@ -211,17 +297,18 @@ fast_read_rgba_pixels_memcpy( struct gl_context *ctx,
    dst = (GLubyte *) _mesa_image_address2d(packing, pixels, width, height,
 					   format, type, 0, 0);
 
-   ctx->Driver.MapRenderbuffer(ctx, rb, x, y, width, height, GL_MAP_READ_BIT,
-			       &map, &stride);
 
-   texelBytes = _mesa_get_format_bytes(rb->Format);
+   map_attachment(ctx, colorAtt, x, y, width, height, GL_MAP_READ_BIT,
+                  &map, &stride);
+
+   texelBytes = _mesa_get_format_bytes(colorFormat);
    for (j = 0; j < height; j++) {
       memcpy(dst, map, width * texelBytes);
       dst += dstStride;
       map += stride;
    }
 
-   ctx->Driver.UnmapRenderbuffer(ctx, rb);
+   unmap_attachment(ctx, colorAtt);
 
    return GL_TRUE;
 }
@@ -235,7 +322,10 @@ slow_read_rgba_pixels( struct gl_context *ctx,
 		       const struct gl_pixelstore_attrib *packing,
 		       GLbitfield transferOps )
 {
-   struct gl_renderbuffer *rb = ctx->ReadBuffer->_ColorReadBuffer;
+   struct gl_framebuffer *fb = ctx->ReadBuffer;
+   struct gl_renderbuffer_attachment *colorAtt =
+      &fb->Attachment[fb->_ColorReadBufferIndex];
+   gl_format colorFormat = get_attachment_format(colorAtt);
    GLfloat rgba[MAX_WIDTH][4];
    GLubyte *dst, *map;
    int dstStride, stride, j;
@@ -244,12 +334,13 @@ slow_read_rgba_pixels( struct gl_context *ctx,
    dst = (GLubyte *) _mesa_image_address2d(packing, pixels, width, height,
 					   format, type, 0, 0);
 
-   ctx->Driver.MapRenderbuffer(ctx, rb, x, y, width, height, GL_MAP_READ_BIT,
-			       &map, &stride);
+   map_attachment(ctx, colorAtt, x, y, width, height, GL_MAP_READ_BIT,
+                  &map, &stride);
+
+   colorFormat = _mesa_get_srgb_format_linear(colorFormat);
 
    for (j = 0; j < height; j++) {
-      _mesa_unpack_rgba_row(_mesa_get_srgb_format_linear(rb->Format),
-			    width, map, rgba);
+      _mesa_unpack_rgba_row(colorFormat, width, map, rgba);
       _mesa_pack_rgba_span_float(ctx, width, rgba, format, type, dst,
 				 packing, transferOps);
 
@@ -257,7 +348,7 @@ slow_read_rgba_pixels( struct gl_context *ctx,
       map += stride;
    }
 
-   ctx->Driver.UnmapRenderbuffer(ctx, rb);
+   unmap_attachment(ctx, colorAtt);
 
    return GL_TRUE;
 }
@@ -308,29 +399,34 @@ fast_read_depth_stencil_pixels(struct gl_context *ctx,
 			       GLubyte *dst, int dstStride)
 {
    struct gl_framebuffer *fb = ctx->ReadBuffer;
-   struct gl_renderbuffer *rb = fb->Attachment[BUFFER_DEPTH].Renderbuffer;
-   struct gl_renderbuffer *stencilRb = fb->Attachment[BUFFER_STENCIL].Renderbuffer;
+   struct gl_renderbuffer_attachment *depthAtt = &fb->Attachment[BUFFER_DEPTH];
+   struct gl_renderbuffer_attachment *stencilAtt = &fb->Attachment[BUFFER_STENCIL];
+   gl_format depthFormat = get_attachment_format(depthAtt);
    GLubyte *map;
    int stride, i;
 
-   if (rb != stencilRb)
+   /* The attachments must either share depth/stencil renderbuffers
+    * or texture objects.
+    */
+   if (depthAtt->Renderbuffer != stencilAtt->Renderbuffer ||
+       depthAtt->Texture != stencilAtt->Texture)
       return GL_FALSE;
 
-   if (rb->Format != MESA_FORMAT_Z24_S8 &&
-       rb->Format != MESA_FORMAT_S8_Z24)
+   if (depthFormat != MESA_FORMAT_Z24_S8 &&
+       depthFormat != MESA_FORMAT_S8_Z24)
       return GL_FALSE;
 
-   ctx->Driver.MapRenderbuffer(ctx, rb, x, y, width, height, GL_MAP_READ_BIT,
-			       &map, &stride);
+   map_attachment(ctx, depthAtt, x, y, width, height, GL_MAP_READ_BIT,
+                  &map, &stride);
 
    for (i = 0; i < height; i++) {
-      _mesa_unpack_uint_24_8_depth_stencil_row(rb->Format, width,
+      _mesa_unpack_uint_24_8_depth_stencil_row(depthFormat, width,
 					       map, (GLuint *)dst);
       map += stride;
       dst += dstStride;
    }
 
-   ctx->Driver.UnmapRenderbuffer(ctx, rb);
+   unmap_attachment(ctx, depthAtt);
 
    return GL_TRUE;
 }
@@ -348,24 +444,26 @@ fast_read_depth_stencil_pixels_separate(struct gl_context *ctx,
 					uint32_t *dst, int dstStride)
 {
    struct gl_framebuffer *fb = ctx->ReadBuffer;
-   struct gl_renderbuffer *depthRb = fb->Attachment[BUFFER_DEPTH].Renderbuffer;
-   struct gl_renderbuffer *stencilRb = fb->Attachment[BUFFER_STENCIL].Renderbuffer;
+   struct gl_renderbuffer_attachment *depthAtt = &fb->Attachment[BUFFER_DEPTH];
+   struct gl_renderbuffer_attachment *stencilAtt = &fb->Attachment[BUFFER_STENCIL];
+   gl_format depthFormat = get_attachment_format(depthAtt);
+   gl_format stencilFormat = get_attachment_format(stencilAtt);
    GLubyte *depthMap, *stencilMap;
    int depthStride, stencilStride, i, j;
 
-   if (_mesa_get_format_datatype(depthRb->Format) != GL_UNSIGNED_INT)
+   if (_mesa_get_format_datatype(depthFormat) != GL_UNSIGNED_INT)
       return GL_FALSE;
 
-   ctx->Driver.MapRenderbuffer(ctx, depthRb, x, y, width, height,
-			       GL_MAP_READ_BIT, &depthMap, &depthStride);
-   ctx->Driver.MapRenderbuffer(ctx, stencilRb, x, y, width, height,
-			       GL_MAP_READ_BIT, &stencilMap, &stencilStride);
+   map_attachment(ctx, depthAtt, x, y, width, height, GL_MAP_READ_BIT,
+                  &depthMap, &depthStride);
+   map_attachment(ctx, stencilAtt, x, y, width, height, GL_MAP_READ_BIT,
+                  &stencilMap, &stencilStride);
 
    for (j = 0; j < height; j++) {
       GLstencil stencilVals[MAX_WIDTH];
 
-      _mesa_unpack_uint_z_row(depthRb->Format, width, depthMap, dst);
-      _mesa_unpack_ubyte_stencil_row(stencilRb->Format, width,
+      _mesa_unpack_uint_z_row(depthFormat, width, depthMap, dst);
+      _mesa_unpack_ubyte_stencil_row(stencilFormat, width,
 				     stencilMap, stencilVals);
 
       for (i = 0; i < width; i++) {
@@ -377,8 +475,8 @@ fast_read_depth_stencil_pixels_separate(struct gl_context *ctx,
       dst += dstStride / 4;
    }
 
-   ctx->Driver.UnmapRenderbuffer(ctx, depthRb);
-   ctx->Driver.UnmapRenderbuffer(ctx, stencilRb);
+   unmap_attachment(ctx, depthAtt);
+   unmap_attachment(ctx, stencilAtt);
 
    return GL_TRUE;
 }
@@ -392,22 +490,24 @@ slow_read_depth_stencil_pixels_separate(struct gl_context *ctx,
 					GLubyte *dst, int dstStride)
 {
    struct gl_framebuffer *fb = ctx->ReadBuffer;
-   struct gl_renderbuffer *depthRb = fb->Attachment[BUFFER_DEPTH].Renderbuffer;
-   struct gl_renderbuffer *stencilRb = fb->Attachment[BUFFER_STENCIL].Renderbuffer;
+   struct gl_renderbuffer_attachment *depthAtt = &fb->Attachment[BUFFER_DEPTH];
+   struct gl_renderbuffer_attachment *stencilAtt = &fb->Attachment[BUFFER_STENCIL];
+   gl_format depthFormat = get_attachment_format(depthAtt);
+   gl_format stencilFormat = get_attachment_format(stencilAtt);
    GLubyte *depthMap, *stencilMap;
    int depthStride, stencilStride, j;
 
-   ctx->Driver.MapRenderbuffer(ctx, depthRb, x, y, width, height,
-			       GL_MAP_READ_BIT, &depthMap, &depthStride);
-   ctx->Driver.MapRenderbuffer(ctx, stencilRb, x, y, width, height,
-			       GL_MAP_READ_BIT, &stencilMap, &stencilStride);
+   map_attachment(ctx, depthAtt, x, y, width, height, GL_MAP_READ_BIT,
+                  &depthMap, &depthStride);
+   map_attachment(ctx, stencilAtt, x, y, width, height, GL_MAP_READ_BIT,
+                  &stencilMap, &stencilStride);
 
    for (j = 0; j < height; j++) {
       GLstencil stencilVals[MAX_WIDTH];
       GLfloat depthVals[MAX_WIDTH];
 
-      _mesa_unpack_float_z_row(depthRb->Format, width, depthMap, depthVals);
-      _mesa_unpack_ubyte_stencil_row(stencilRb->Format, width,
+      _mesa_unpack_float_z_row(depthFormat, width, depthMap, depthVals);
+      _mesa_unpack_ubyte_stencil_row(stencilFormat, width,
 				     stencilMap, stencilVals);
 
       _mesa_pack_depth_stencil_span(ctx, width, type, (GLuint *)dst,
@@ -418,8 +518,8 @@ slow_read_depth_stencil_pixels_separate(struct gl_context *ctx,
       dst += dstStride;
    }
 
-   ctx->Driver.UnmapRenderbuffer(ctx, depthRb);
-   ctx->Driver.UnmapRenderbuffer(ctx, stencilRb);
+   unmap_attachment(ctx, depthAtt);
+   unmap_attachment(ctx, stencilAtt);
 }
 
 
-- 
1.7.3.4



More information about the mesa-dev mailing list