mesa: Pass GL context to _mesa_create_save_table
[mesa.git] / src / mesa / main / readpix.c
index 8048a7286df911a13b4b182d10c6d8e557ab5cec..f6680c91f44171adacc5df9dd044556e140703b4 100644 (file)
@@ -36,6 +36,8 @@
 #include "pack.h"
 #include "pbo.h"
 #include "state.h"
+#include "glformats.h"
+#include "fbobject.h"
 
 
 /**
@@ -60,7 +62,7 @@ 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(rb->Format) != GL_UNSIGNED_NORMALIZED)
       return GL_FALSE;
 
    if (!((type == GL_UNSIGNED_SHORT && rb->Format == MESA_FORMAT_Z16) ||
@@ -110,6 +112,7 @@ read_depth_pixels( struct gl_context *ctx,
    GLint j;
    GLubyte *dst, *map;
    int dstStride, stride;
+   GLfloat *depthValues;
 
    if (!rb)
       return;
@@ -119,8 +122,6 @@ read_depth_pixels( struct gl_context *ctx,
    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);
 
    if (fast_read_depth_pixels(ctx, x, y, width, height, type, pixels, packing))
       return;
@@ -136,16 +137,24 @@ read_depth_pixels( struct gl_context *ctx,
       return;
    }
 
-   /* 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_pack_depth_span(ctx, width, dst, type, depthValues, packing);
+   depthValues = malloc(width * sizeof(GLfloat));
 
-      dst += dstStride;
-      map += stride;
+   if (depthValues) {
+      /* General case (slower) */
+      for (j = 0; j < height; j++, y++) {
+         _mesa_unpack_float_z_row(rb->Format, width, map, depthValues);
+         _mesa_pack_depth_span(ctx, width, dst, type, depthValues, packing);
+
+         dst += dstStride;
+         map += stride;
+      }
+   }
+   else {
+      _mesa_error(ctx, GL_OUT_OF_MEMORY, "glReadPixels");
    }
 
+   free(depthValues);
+
    ctx->Driver.UnmapRenderbuffer(ctx, rb);
 }
 
@@ -163,15 +172,12 @@ read_stencil_pixels( struct gl_context *ctx,
    struct gl_framebuffer *fb = ctx->ReadBuffer;
    struct gl_renderbuffer *rb = fb->Attachment[BUFFER_STENCIL].Renderbuffer;
    GLint j;
-   GLubyte *map;
+   GLubyte *map, *stencil;
    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);
    if (!map) {
@@ -179,23 +185,36 @@ read_stencil_pixels( struct gl_context *ctx,
       return;
    }
 
-   /* process image row by row */
-   for (j = 0; j < height; j++) {
-      GLvoid *dest;
-      GLubyte stencil[MAX_WIDTH];
+   stencil = malloc(width * sizeof(GLubyte));
 
-      _mesa_unpack_ubyte_stencil_row(rb->Format, width, map, stencil);
-      dest = _mesa_image_address2d(packing, pixels, width, height,
-                                   GL_STENCIL_INDEX, type, j, 0);
+   if (stencil) {
+      /* process image row by row */
+      for (j = 0; j < height; j++) {
+         GLvoid *dest;
 
-      _mesa_pack_stencil_span(ctx, width, type, dest, stencil, packing);
+         _mesa_unpack_ubyte_stencil_row(rb->Format, width, map, stencil);
+         dest = _mesa_image_address2d(packing, pixels, width, height,
+                                      GL_STENCIL_INDEX, type, j, 0);
 
-      map += stride;
+         _mesa_pack_stencil_span(ctx, width, type, dest, stencil, packing);
+
+         map += stride;
+      }
+   }
+   else {
+      _mesa_error(ctx, GL_OUT_OF_MEMORY, "glReadPixels");
    }
 
+   free(stencil);
+
    ctx->Driver.UnmapRenderbuffer(ctx, rb);
 }
 
+
+/**
+ * Try to do glReadPixels of RGBA data using a simple memcpy or swizzle.
+ * \return GL_TRUE if successful, GL_FALSE otherwise (use the slow path)
+ */
 static GLboolean
 fast_read_rgba_pixels_memcpy( struct gl_context *ctx,
                              GLint x, GLint y,
@@ -208,15 +227,34 @@ fast_read_rgba_pixels_memcpy( struct gl_context *ctx,
    struct gl_renderbuffer *rb = ctx->ReadBuffer->_ColorReadBuffer;
    GLubyte *dst, *map;
    int dstStride, stride, j, texelBytes;
-
-   if (!_mesa_format_matches_format_and_type(rb->Format, format, type))
+   GLboolean swizzle_rb = GL_FALSE, copy_xrgb = GL_FALSE;
+
+   /* XXX we could check for other swizzle/special cases here as needed */
+   if (rb->Format == MESA_FORMAT_RGBA8888_REV &&
+       format == GL_BGRA &&
+       type == GL_UNSIGNED_INT_8_8_8_8_REV &&
+       !ctx->Pack.SwapBytes) {
+      swizzle_rb = GL_TRUE;
+   }
+   else if (rb->Format == MESA_FORMAT_XRGB8888 &&
+       format == GL_BGRA &&
+       type == GL_UNSIGNED_INT_8_8_8_8_REV &&
+       !ctx->Pack.SwapBytes) {
+      copy_xrgb = GL_TRUE;
+   }
+   else if (!_mesa_format_matches_format_and_type(rb->Format, format, type,
+                                                  ctx->Pack.SwapBytes))
       return GL_FALSE;
 
-   /* check for things we can't handle here */
-   if (packing->SwapBytes ||
-       packing->LsbFirst) {
+   /* If the format is unsigned normalized then we can ignore clamping
+    * because the values are already in the range [0,1] so it won't
+    * have any effect anyway.
+    */
+   if (_mesa_get_format_datatype(rb->Format) == GL_UNSIGNED_NORMALIZED)
+      transferOps &= ~IMAGE_CLAMP_BIT;
+
+   if (transferOps)
       return GL_FALSE;
-   }
 
    dstStride = _mesa_image_row_stride(packing, width, format, type);
    dst = (GLubyte *) _mesa_image_address2d(packing, pixels, width, height,
@@ -230,10 +268,39 @@ fast_read_rgba_pixels_memcpy( struct gl_context *ctx,
    }
 
    texelBytes = _mesa_get_format_bytes(rb->Format);
-   for (j = 0; j < height; j++) {
-      memcpy(dst, map, width * texelBytes);
-      dst += dstStride;
-      map += stride;
+
+   if (swizzle_rb) {
+      /* swap R/B */
+      for (j = 0; j < height; j++) {
+         int i;
+         for (i = 0; i < width; i++) {
+            GLuint *dst4 = (GLuint *) dst, *map4 = (GLuint *) map;
+            GLuint pixel = map4[i];
+            dst4[i] = (pixel & 0xff00ff00)
+                   | ((pixel & 0x00ff0000) >> 16)
+                   | ((pixel & 0x000000ff) << 16);
+         }
+         dst += dstStride;
+         map += stride;
+      }
+   } else if (copy_xrgb) {
+      /* convert xrgb -> argb */
+      for (j = 0; j < height; j++) {
+         GLuint *dst4 = (GLuint *) dst, *map4 = (GLuint *) map;
+         int i;
+         for (i = 0; i < width; i++) {
+            dst4[i] = map4[i] | 0xff000000;  /* set A=0xff */
+         }
+         dst += dstStride;
+         map += stride;
+      }
+   } else {
+      /* just memcpy */
+      for (j = 0; j < height; j++) {
+         memcpy(dst, map, width * texelBytes);
+         dst += dstStride;
+         map += stride;
+      }
    }
 
    ctx->Driver.UnmapRenderbuffer(ctx, rb);
@@ -252,12 +319,11 @@ slow_read_rgba_pixels( struct gl_context *ctx,
 {
    struct gl_renderbuffer *rb = ctx->ReadBuffer->_ColorReadBuffer;
    const gl_format rbFormat = _mesa_get_srgb_format_linear(rb->Format);
-   union {
-      float f[MAX_WIDTH][4];
-      unsigned int i[MAX_WIDTH][4];
-   } rgba;
+   void *rgba;
    GLubyte *dst, *map;
    int dstStride, stride, j;
+   GLboolean dst_is_integer = _mesa_is_enum_format_integer(format);
+   GLboolean dst_is_uint = _mesa_is_format_unsigned(rbFormat);
 
    dstStride = _mesa_image_row_stride(packing, width, format, type);
    dst = (GLubyte *) _mesa_image_address2d(packing, pixels, width, height,
@@ -270,19 +336,36 @@ slow_read_rgba_pixels( struct gl_context *ctx,
       return;
    }
 
+   rgba = malloc(width * MAX_PIXEL_BYTES);
+   if (!rgba)
+      goto done;
+
    for (j = 0; j < height; j++) {
-      if (_mesa_is_integer_format(format)) {
-        _mesa_unpack_int_rgba_row(rbFormat, width, map, rgba.i);
-        _mesa_pack_rgba_span_int(ctx, width, rgba.i, format, type, dst);
+      if (dst_is_integer) {
+        _mesa_unpack_uint_rgba_row(rbFormat, width, map, (GLuint (*)[4]) rgba);
+         _mesa_rebase_rgba_uint(width, (GLuint (*)[4]) rgba,
+                                rb->_BaseFormat);
+         if (dst_is_uint) {
+            _mesa_pack_rgba_span_from_uints(ctx, width, (GLuint (*)[4]) rgba, format,
+                                            type, dst);
+         } else {
+            _mesa_pack_rgba_span_from_ints(ctx, width, (GLint (*)[4]) rgba, format,
+                                           type, dst);
+         }
       } else {
-        _mesa_unpack_rgba_row(rbFormat, width, map, rgba.f);
-        _mesa_pack_rgba_span_float(ctx, width, rgba.f, format, type, dst,
-                                   packing, transferOps);
+        _mesa_unpack_rgba_row(rbFormat, width, map, (GLfloat (*)[4]) rgba);
+         _mesa_rebase_rgba_float(width, (GLfloat (*)[4]) rgba,
+                                 rb->_BaseFormat);
+        _mesa_pack_rgba_span_float(ctx, width, (GLfloat (*)[4]) rgba, format,
+                                    type, dst, packing, transferOps);
       }
       dst += dstStride;
       map += stride;
    }
 
+   free(rgba);
+
+done:
    ctx->Driver.UnmapRenderbuffer(ctx, rb);
 }
 
@@ -304,17 +387,15 @@ read_rgba_pixels( struct gl_context *ctx,
       return;
 
    if ((ctx->Color._ClampReadColor == GL_TRUE || type != GL_FLOAT) &&
-       !_mesa_is_integer_format(format)) {
+       !_mesa_is_enum_format_integer(format)) {
       transferOps |= IMAGE_CLAMP_BIT;
    }
 
-   if (!transferOps) {
-      /* Try the optimized paths first. */
-      if (fast_read_rgba_pixels_memcpy(ctx, x, y, width, height,
-                                      format, type, pixels, packing,
-                                      transferOps)) {
-        return;
-      }
+   /* Try the optimized paths first. */
+   if (fast_read_rgba_pixels_memcpy(ctx, x, y, width, height,
+                                    format, type, pixels, packing,
+                                    transferOps)) {
+      return;
    }
 
    slow_read_rgba_pixels(ctx, x, y, width, height,
@@ -378,10 +459,10 @@ fast_read_depth_stencil_pixels_separate(struct gl_context *ctx,
    struct gl_framebuffer *fb = ctx->ReadBuffer;
    struct gl_renderbuffer *depthRb = fb->Attachment[BUFFER_DEPTH].Renderbuffer;
    struct gl_renderbuffer *stencilRb = fb->Attachment[BUFFER_STENCIL].Renderbuffer;
-   GLubyte *depthMap, *stencilMap;
+   GLubyte *depthMap, *stencilMap, *stencilVals;
    int depthStride, stencilStride, i, j;
 
-   if (_mesa_get_format_datatype(depthRb->Format) != GL_UNSIGNED_INT)
+   if (_mesa_get_format_datatype(depthRb->Format) != GL_UNSIGNED_NORMALIZED)
       return GL_FALSE;
 
    ctx->Driver.MapRenderbuffer(ctx, depthRb, x, y, width, height,
@@ -399,22 +480,29 @@ fast_read_depth_stencil_pixels_separate(struct gl_context *ctx,
       return GL_TRUE;  /* don't bother trying the slow path */
    }
 
-   for (j = 0; j < height; j++) {
-      GLubyte stencilVals[MAX_WIDTH];
+   stencilVals = malloc(width * sizeof(GLubyte));
 
-      _mesa_unpack_uint_z_row(depthRb->Format, width, depthMap, dst);
-      _mesa_unpack_ubyte_stencil_row(stencilRb->Format, width,
-                                    stencilMap, stencilVals);
+   if (stencilVals) {
+      for (j = 0; j < height; j++) {
+         _mesa_unpack_uint_z_row(depthRb->Format, width, depthMap, dst);
+         _mesa_unpack_ubyte_stencil_row(stencilRb->Format, width,
+                                        stencilMap, stencilVals);
 
-      for (i = 0; i < width; i++) {
-        dst[i] = (dst[i] & 0xffffff00) | stencilVals[i];
-      }
+         for (i = 0; i < width; i++) {
+            dst[i] = (dst[i] & 0xffffff00) | stencilVals[i];
+         }
 
-      depthMap += depthStride;
-      stencilMap += stencilStride;
-      dst += dstStride / 4;
+         depthMap += depthStride;
+         stencilMap += stencilStride;
+         dst += dstStride / 4;
+      }
+   }
+   else {
+      _mesa_error(ctx, GL_OUT_OF_MEMORY, "glReadPixels");
    }
 
+   free(stencilVals);
+
    ctx->Driver.UnmapRenderbuffer(ctx, depthRb);
    ctx->Driver.UnmapRenderbuffer(ctx, stencilRb);
 
@@ -434,6 +522,9 @@ slow_read_depth_stencil_pixels_separate(struct gl_context *ctx,
    struct gl_renderbuffer *stencilRb = fb->Attachment[BUFFER_STENCIL].Renderbuffer;
    GLubyte *depthMap, *stencilMap;
    int depthStride, stencilStride, j;
+   GLubyte *stencilVals;
+   GLfloat *depthVals;
+
 
    /* The depth and stencil buffers might be separate, or a single buffer.
     * If one buffer, only map it once.
@@ -460,21 +551,29 @@ slow_read_depth_stencil_pixels_separate(struct gl_context *ctx,
       stencilStride = depthStride;
    }
 
-   for (j = 0; j < height; j++) {
-      GLubyte stencilVals[MAX_WIDTH];
-      GLfloat depthVals[MAX_WIDTH];
+   stencilVals = malloc(width * sizeof(GLubyte));
+   depthVals = malloc(width * sizeof(GLfloat));
 
-      _mesa_unpack_float_z_row(depthRb->Format, width, depthMap, depthVals);
-      _mesa_unpack_ubyte_stencil_row(stencilRb->Format, width,
-                                    stencilMap, stencilVals);
+   if (stencilVals && depthVals) {
+      for (j = 0; j < height; j++) {
+         _mesa_unpack_float_z_row(depthRb->Format, width, depthMap, depthVals);
+         _mesa_unpack_ubyte_stencil_row(stencilRb->Format, width,
+                                        stencilMap, stencilVals);
 
-      _mesa_pack_depth_stencil_span(ctx, width, type, (GLuint *)dst,
-                                   depthVals, stencilVals, packing);
+         _mesa_pack_depth_stencil_span(ctx, width, type, (GLuint *)dst,
+                                       depthVals, stencilVals, packing);
 
-      depthMap += depthStride;
-      stencilMap += stencilStride;
-      dst += dstStride;
+         depthMap += depthStride;
+         stencilMap += stencilStride;
+         dst += dstStride;
+      }
    }
+   else {
+      _mesa_error(ctx, GL_OUT_OF_MEMORY, "glReadPixels");
+   }
+
+   free(stencilVals);
+   free(depthVals);
 
    ctx->Driver.UnmapRenderbuffer(ctx, depthRb);
    if (stencilRb != depthRb) {
@@ -575,162 +674,13 @@ _mesa_readpixels(struct gl_context *ctx,
 }
 
 
-/**
- * Do error checking of the format/type parameters to glReadPixels and
- * glDrawPixels.
- * \param drawing if GL_TRUE do checking for DrawPixels, else do checking
- *                for ReadPixels.
- * \return GL_TRUE if error detected, GL_FALSE if no errors
- */
-GLboolean
-_mesa_error_check_format_type(struct gl_context *ctx, GLenum format,
-                              GLenum type, GLboolean drawing)
-{
-   const char *readDraw = drawing ? "Draw" : "Read";
-   const GLboolean reading = !drawing;
-
-   /* state validation should have already been done */
-   ASSERT(ctx->NewState == 0x0);
-
-   if (ctx->Extensions.EXT_packed_depth_stencil
-       && type == GL_UNSIGNED_INT_24_8_EXT
-       && format != GL_DEPTH_STENCIL_EXT) {
-      _mesa_error(ctx, GL_INVALID_OPERATION,
-                  "gl%sPixels(format is not GL_DEPTH_STENCIL_EXT)", readDraw);
-      return GL_TRUE;
-   }
-
-   if (ctx->Extensions.ARB_depth_buffer_float
-       && type == GL_FLOAT_32_UNSIGNED_INT_24_8_REV
-       && format != GL_DEPTH_STENCIL_EXT) {
-      _mesa_error(ctx, GL_INVALID_OPERATION,
-                  "gl%sPixels(format is not GL_DEPTH_STENCIL_EXT)", readDraw);
-      return GL_TRUE;
-   }
-
-   /* basic combinations test */
-   if (!_mesa_is_legal_format_and_type(ctx, format, type)) {
-      _mesa_error(ctx, GL_INVALID_ENUM,
-                  "gl%sPixels(format or type)", readDraw);
-      return GL_TRUE;
-   }
-
-   /* additional checks */
-   switch (format) {
-   case GL_RG:
-   case GL_RED:
-   case GL_GREEN:
-   case GL_BLUE:
-   case GL_ALPHA:
-   case GL_LUMINANCE:
-   case GL_LUMINANCE_ALPHA:
-   case GL_RGB:
-   case GL_BGR:
-   case GL_RGBA:
-   case GL_BGRA:
-   case GL_ABGR_EXT:
-   case GL_RED_INTEGER_EXT:
-   case GL_GREEN_INTEGER_EXT:
-   case GL_BLUE_INTEGER_EXT:
-   case GL_ALPHA_INTEGER_EXT:
-   case GL_RGB_INTEGER_EXT:
-   case GL_RGBA_INTEGER_EXT:
-   case GL_BGR_INTEGER_EXT:
-   case GL_BGRA_INTEGER_EXT:
-   case GL_LUMINANCE_INTEGER_EXT:
-   case GL_LUMINANCE_ALPHA_INTEGER_EXT:
-      if (!drawing) {
-         /* reading */
-         if (!_mesa_source_buffer_exists(ctx, GL_COLOR)) {
-            _mesa_error(ctx, GL_INVALID_OPERATION,
-                        "glReadPixels(no color buffer)");
-            return GL_TRUE;
-         }
-      }
-      break;
-   case GL_COLOR_INDEX:
-      if (drawing) {
-         if (ctx->PixelMaps.ItoR.Size == 0 ||
-            ctx->PixelMaps.ItoG.Size == 0 ||
-            ctx->PixelMaps.ItoB.Size == 0) {
-            _mesa_error(ctx, GL_INVALID_OPERATION,
-                   "glDrawPixels(drawing color index pixels into RGB buffer)");
-            return GL_TRUE;
-         }
-      }
-      else {
-         /* reading */
-         if (!_mesa_source_buffer_exists(ctx, GL_COLOR)) {
-            _mesa_error(ctx, GL_INVALID_OPERATION,
-                        "glReadPixels(no color buffer)");
-            return GL_TRUE;
-         }
-         /* We no longer support CI-mode color buffers so trying to read
-          * GL_COLOR_INDEX pixels is always an error.
-          */
-         _mesa_error(ctx, GL_INVALID_OPERATION,
-                     "glReadPixels(color buffer is RGB)");
-         return GL_TRUE;
-      }
-      break;
-   case GL_STENCIL_INDEX:
-      if ((drawing && !_mesa_dest_buffer_exists(ctx, format)) ||
-          (reading && !_mesa_source_buffer_exists(ctx, format))) {
-         _mesa_error(ctx, GL_INVALID_OPERATION,
-                     "gl%sPixels(no stencil buffer)", readDraw);
-         return GL_TRUE;
-      }
-      break;
-   case GL_DEPTH_COMPONENT:
-      if ((drawing && !_mesa_dest_buffer_exists(ctx, format))) {
-         _mesa_error(ctx, GL_INVALID_OPERATION,
-                     "gl%sPixels(no depth buffer)", readDraw);
-         return GL_TRUE;
-      }
-      break;
-   case GL_DEPTH_STENCIL_EXT:
-      /* Check validity of the type first. */
-      switch (type) {
-         case GL_UNSIGNED_INT_24_8_EXT:
-            if (!ctx->Extensions.EXT_packed_depth_stencil) {
-               _mesa_error(ctx, GL_INVALID_ENUM, "gl%sPixels(type)", readDraw);
-               return GL_TRUE;
-            }
-            break;
-         case GL_FLOAT_32_UNSIGNED_INT_24_8_REV:
-            if (!ctx->Extensions.ARB_depth_buffer_float) {
-               _mesa_error(ctx, GL_INVALID_ENUM, "gl%sPixels(type)", readDraw);
-               return GL_TRUE;
-            }
-            break;
-         default:
-            _mesa_error(ctx, GL_INVALID_ENUM, "gl%sPixels(type)", readDraw);
-            return GL_TRUE;
-      }
-      if ((drawing && !_mesa_dest_buffer_exists(ctx, format)) ||
-          (reading && !_mesa_source_buffer_exists(ctx, format))) {
-         _mesa_error(ctx, GL_INVALID_OPERATION,
-                     "gl%sPixels(no depth or stencil buffer)", readDraw);
-         return GL_TRUE;
-      }
-      break;
-   default:
-      /* this should have been caught in _mesa_is_legal_format_type() */
-      _mesa_problem(ctx, "unexpected format in _mesa_%sPixels", readDraw);
-      return GL_TRUE;
-   }
-
-   /* no errors */
-   return GL_FALSE;
-}
-      
-
-
 void GLAPIENTRY
 _mesa_ReadnPixelsARB( GLint x, GLint y, GLsizei width, GLsizei height,
                      GLenum format, GLenum type, GLsizei bufSize,
                       GLvoid *pixels )
 {
+   GLenum err;
+
    GET_CURRENT_CONTEXT(ctx);
    ASSERT_OUTSIDE_BEGIN_END_AND_FLUSH(ctx);
 
@@ -749,11 +699,47 @@ _mesa_ReadnPixelsARB( GLint x, GLint y, GLsizei width, GLsizei height,
       return;
    }
 
+   /* OpenGL ES 1.x and OpenGL ES 2.0 impose additional restrictions on the
+    * combinations of format and type that can be used.
+    *
+    * Technically, only two combinations are actually allowed:
+    * GL_RGBA/GL_UNSIGNED_BYTE, and some implementation-specific internal
+    * preferred combination.  This code doesn't know what that preferred
+    * combination is, and Mesa can handle anything valid.  Just work instead.
+    */
+   if (_mesa_is_gles(ctx) && ctx->Version < 30) {
+      err = _mesa_es_error_check_format_and_type(format, type, 2);
+      if (err == GL_NO_ERROR) {
+         if (type == GL_FLOAT || type == GL_HALF_FLOAT_OES) {
+            err = GL_INVALID_OPERATION;
+         } else if (format == GL_DEPTH_COMPONENT
+                    || format == GL_DEPTH_STENCIL) {
+            err = GL_INVALID_ENUM;
+         }
+      }
+
+      if (err != GL_NO_ERROR) {
+         _mesa_error(ctx, err, "glReadPixels(invalid format %s and/or type %s)",
+                     _mesa_lookup_enum_by_nr(format),
+                     _mesa_lookup_enum_by_nr(type));
+         return;
+      }
+   }
+
    if (ctx->NewState)
       _mesa_update_state(ctx);
 
-   if (_mesa_error_check_format_type(ctx, format, type, GL_FALSE)) {
-      /* found an error */
+   err = _mesa_error_check_format_and_type(ctx, format, type);
+   if (err != GL_NO_ERROR) {
+      _mesa_error(ctx, err, "glReadPixels(invalid format %s and/or type %s)",
+                  _mesa_lookup_enum_by_nr(format),
+                  _mesa_lookup_enum_by_nr(type));
+      return;
+   }
+
+   if (ctx->ReadBuffer->_Status != GL_FRAMEBUFFER_COMPLETE_EXT) {
+      _mesa_error(ctx, GL_INVALID_FRAMEBUFFER_OPERATION_EXT,
+                  "glReadPixels(incomplete framebuffer)" );
       return;
    }
 
@@ -763,7 +749,7 @@ _mesa_ReadnPixelsARB( GLint x, GLint y, GLsizei width, GLsizei height,
    if (ctx->Extensions.EXT_texture_integer && _mesa_is_color_format(format)) {
       const struct gl_renderbuffer *rb = ctx->ReadBuffer->_ColorReadBuffer;
       const GLboolean srcInteger = _mesa_is_format_integer_color(rb->Format);
-      const GLboolean dstInteger = _mesa_is_integer_format(format);
+      const GLboolean dstInteger = _mesa_is_enum_format_integer(format);
       if (dstInteger != srcInteger) {
          _mesa_error(ctx, GL_INVALID_OPERATION,
                      "glReadPixels(integer / non-integer format mismatch");
@@ -771,9 +757,9 @@ _mesa_ReadnPixelsARB( GLint x, GLint y, GLsizei width, GLsizei height,
       }
    }
 
-   if (ctx->ReadBuffer->_Status != GL_FRAMEBUFFER_COMPLETE_EXT) {
-      _mesa_error(ctx, GL_INVALID_FRAMEBUFFER_OPERATION_EXT,
-                  "glReadPixels(incomplete framebuffer)" );
+   if (_mesa_is_user_fbo(ctx->ReadBuffer) &&
+       ctx->ReadBuffer->Visual.samples > 0) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glReadPixels(multisample FBO)");
       return;
    }