Lift all the format/type error checking out of the _swrast_Draw/Read/CopyPixels

functions into the _mesa_Draw/Read/CopyPixels functions.

4 files changed, 148 insertions, 173 deletions

diff --git a/src/mesa/main/drawpix.c b/src/mesa/main/drawpix.c
index 81a5bb0efc..3ced207247 100644
--- a/src/mesa/main/drawpix.c
+++ b/src/mesa/main/drawpix.c
@@ -24,13 +24,83 @@
 
 #include "glheader.h"
 #include "imports.h"
-#include "colormac.h"
 #include "context.h"
 #include "drawpix.h"
 #include "feedback.h"
-#include "macros.h"
+#include "image.h"
 #include "state.h"
-#include "mtypes.h"
+
+
+/**
+ * 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
+ */
+static GLboolean
+error_check_format_type(GLcontext *ctx, GLenum format, GLenum type,
+                        GLboolean drawing)
+{
+   const char *readDraw = drawing ? "Draw" : "Read";
+
+   /* 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_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:
+      if (drawing && !ctx->Visual.rgbMode) {
+         _mesa_error(ctx, GL_INVALID_OPERATION,
+                   "glDrawPixels(drawing RGB pixels into color index buffer)");
+         return GL_TRUE;
+      }
+      break;
+   case GL_COLOR_INDEX:
+      if (!drawing && ctx->Visual.rgbMode) {
+         _mesa_error(ctx, GL_INVALID_OPERATION,
+                    "glReadPixels(reading color index format from RGB buffer");
+         return GL_TRUE;
+      }
+      break;
+   case GL_STENCIL_INDEX:
+      if (ctx->DrawBuffer->Visual.stencilBits == 0) {
+         _mesa_error(ctx, GL_INVALID_OPERATION,
+                     "gl%sPixels(no stencil buffer)", readDraw);
+         return GL_TRUE;
+      }
+      break;
+   case GL_DEPTH_COMPONENT:
+      if (ctx->DrawBuffer->Visual.depthBits == 0) {
+         _mesa_error(ctx, GL_INVALID_OPERATION,
+                     "gl%sPixels(no depth 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;
+}
+      
 
 
 #if _HAVE_FULL_GL
@@ -56,10 +126,15 @@ _mesa_DrawPixels( GLsizei width, GLsizei height,
       return;
    }
 
-   if (!ctx->Current.RasterPosValid) {
+   if (error_check_format_type(ctx, format, type, GL_TRUE)) {
+      /* found an error */
       return;
    }
 
+   if (!ctx->Current.RasterPosValid) {
+      return; /* not an error */
+   }
+
    if (ctx->NewState) {
       _mesa_update_state(ctx);
    }
@@ -102,7 +177,32 @@ _mesa_CopyPixels( GLint srcx, GLint srcy, GLsizei width, GLsizei height,
    }
 
    if (width < 0 || height < 0) {
-      _mesa_error( ctx, GL_INVALID_VALUE, "glCopyPixels(width or height < 0)" );
+      _mesa_error(ctx, GL_INVALID_VALUE, "glCopyPixels(width or height < 0)");
+      return;
+   }
+
+   switch (type) {
+   case GL_COLOR:
+      /* OK */
+      break;
+   case GL_DEPTH:
+      if (ctx->DrawBuffer->Visual.depthBits == 0 ||
+          ctx->ReadBuffer->Visual.depthBits == 0) {
+         _mesa_error(ctx, GL_INVALID_OPERATION,
+                     "glCopyPixels(no depth buffer)");
+         return;
+      }
+      break;
+   case GL_STENCIL:
+      if (ctx->DrawBuffer->Visual.stencilBits == 0 ||
+          ctx->ReadBuffer->Visual.stencilBits == 0) {
+         _mesa_error(ctx, GL_INVALID_OPERATION,
+                     "glCopyPixels(no stencil buffer)");
+         return;
+      }
+      break;
+   default:
+      _mesa_error(ctx, GL_INVALID_ENUM, "glCopyPixels");
       return;
    }
 
@@ -153,6 +253,11 @@ _mesa_ReadPixels( GLint x, GLint y, GLsizei width, GLsizei height,
       return;
    }
 
+   if (error_check_format_type(ctx, format, type, GL_FALSE)) {
+      /* found an error */
+      return;
+   }
+
    if (ctx->NewState)
       _mesa_update_state(ctx);
 
diff --git a/src/mesa/swrast/s_copypix.c b/src/mesa/swrast/s_copypix.c
index 2b4e8450d9..ed6a02ee13 100644
--- a/src/mesa/swrast/s_copypix.c
+++ b/src/mesa/swrast/s_copypix.c
@@ -517,11 +517,6 @@ copy_depth_pixels( GLcontext *ctx, GLint srcx, GLint srcy,
 
    INIT_SPAN(span, GL_BITMAP, 0, 0, SPAN_Z);
 
-   if (fb->Visual.depthBits == 0) {
-      _mesa_error( ctx, GL_INVALID_OPERATION, "glCopyPixels" );
-      return;
-   }
-
    /* Determine if copy should be bottom-to-top or top-to-bottom */
    if (srcy<desty) {
       /* top-down  max-to-min */
@@ -623,11 +618,6 @@ copy_stencil_pixels( GLcontext *ctx, GLint srcx, GLint srcy,
    const GLboolean shift_or_offset = ctx->Pixel.IndexShift || ctx->Pixel.IndexOffset;
    GLint overlapping;
 
-   if (fb->Visual.stencilBits == 0) {
-      _mesa_error( ctx, GL_INVALID_OPERATION, "glCopyPixels" );
-      return;
-   }
-
    if (!rb) {
       /* no readbuffer - OK */
       return;
@@ -709,12 +699,14 @@ copy_stencil_pixels( GLcontext *ctx, GLint srcx, GLint srcy,
 }
 
 
-
+/**
+ * Do software-based glCopyPixels.
+ * By time we get here, all parameters will have been error-checked.
+ */
 void
 _swrast_CopyPixels( GLcontext *ctx,
 		    GLint srcx, GLint srcy, GLsizei width, GLsizei height,
-		    GLint destx, GLint desty,
-		    GLenum type )
+		    GLint destx, GLint desty, GLenum type )
 {
    SWcontext *swrast = SWRAST_CONTEXT(ctx);
    RENDER_START(swrast,ctx);
@@ -722,20 +714,23 @@ _swrast_CopyPixels( GLcontext *ctx,
    if (swrast->NewState)
       _swrast_validate_derived( ctx );
 
-   if (type == GL_COLOR && ctx->Visual.rgbMode) {
-      copy_rgba_pixels( ctx, srcx, srcy, width, height, destx, desty );
-   }
-   else if (type == GL_COLOR && !ctx->Visual.rgbMode) {
-      copy_ci_pixels( ctx, srcx, srcy, width, height, destx, desty );
-   }
-   else if (type == GL_DEPTH) {
+   switch (type) {
+   case GL_COLOR:
+      if (ctx->Visual.rgbMode) {
+         copy_rgba_pixels( ctx, srcx, srcy, width, height, destx, desty );
+      }
+      else {
+         copy_ci_pixels( ctx, srcx, srcy, width, height, destx, desty );
+      }
+      break;
+   case GL_DEPTH:
       copy_depth_pixels( ctx, srcx, srcy, width, height, destx, desty );
-   }
-   else if (type == GL_STENCIL) {
+      break;
+   case GL_STENCIL:
       copy_stencil_pixels( ctx, srcx, srcy, width, height, destx, desty );
-   }
-   else {
-      _mesa_error( ctx, GL_INVALID_ENUM, "glCopyPixels" );
+      break;
+   default:
+      _mesa_problem(ctx, "unexpected type in _swrast_CopyPixels");
    }
 
    RENDER_FINISH(swrast,ctx);
diff --git a/src/mesa/swrast/s_drawpix.c b/src/mesa/swrast/s_drawpix.c
index 91ac13b13b..d908ca4db4 100644
--- a/src/mesa/swrast/s_drawpix.c
+++ b/src/mesa/swrast/s_drawpix.c
@@ -517,23 +517,6 @@ draw_stencil_pixels( GLcontext *ctx, GLint x, GLint y,
    const GLint desty = y;
    GLint row, skipPixels;
 
-   if (type != GL_BYTE &&
-       type != GL_UNSIGNED_BYTE &&
-       type != GL_SHORT &&
-       type != GL_UNSIGNED_SHORT &&
-       type != GL_INT &&
-       type != GL_UNSIGNED_INT &&
-       type != GL_FLOAT &&
-       type != GL_BITMAP) {
-      _mesa_error( ctx, GL_INVALID_ENUM, "glDrawPixels(stencil type)");
-      return;
-   }
-
-   if (ctx->DrawBuffer->Visual.stencilBits == 0) {
-      _mesa_error(ctx, GL_INVALID_OPERATION, "glDrawPixels(no stencil buffer)");
-      return;
-   }
-
    /* if width > MAX_WIDTH, have to process image in chunks */
    skipPixels = 0;
    while (skipPixels < width) {
@@ -565,8 +548,7 @@ draw_stencil_pixels( GLcontext *ctx, GLint x, GLint y,
                                             spanX, spanY, values, desty, 0);
          }
          else {
-            _swrast_write_stencil_span(ctx, (GLuint) spanWidth,
-                                     spanX, spanY, values);
+            _swrast_write_stencil_span(ctx, spanWidth, spanX, spanY, values);
          }
       }
       skipPixels += spanWidth;
@@ -592,17 +574,6 @@ draw_depth_pixels( GLcontext *ctx, GLint x, GLint y,
 
    INIT_SPAN(span, GL_BITMAP, 0, 0, SPAN_Z);
 
-   if (type != GL_BYTE
-       && type != GL_UNSIGNED_BYTE
-       && type != GL_SHORT
-       && type != GL_UNSIGNED_SHORT
-       && type != GL_INT
-       && type != GL_UNSIGNED_INT
-       && type != GL_FLOAT) {
-      _mesa_error(ctx, GL_INVALID_ENUM, "glDrawPixels(type)");
-      return;
-   }
-
    _swrast_span_default_color(ctx, &span);
 
    if (ctx->Fog.Enabled)
@@ -729,11 +700,6 @@ draw_rgba_pixels( GLcontext *ctx, GLint x, GLint y,
 
    INIT_SPAN(span, GL_BITMAP, 0, 0, SPAN_RGBA);
 
-   if (!_mesa_is_legal_format_and_type(ctx, format, type)) {
-      _mesa_error(ctx, GL_INVALID_ENUM, "glDrawPixels(format or type)");
-      return;
-   }
-
    /* Try an optimized glDrawPixels first */
    if (fast_draw_pixels(ctx, x, y, width, height, format, type, unpack, pixels))
       return;
@@ -874,9 +840,9 @@ draw_rgba_pixels( GLcontext *ctx, GLint x, GLint y,
 }
 
 
-
-/*
- * Execute glDrawPixels
+/**
+ * Execute software-based glDrawPixels.
+ * By time we get here, all error checking will have been done.
  */
 void
 _swrast_DrawPixels( GLcontext *ctx,
@@ -940,7 +906,7 @@ _swrast_DrawPixels( GLcontext *ctx,
       draw_rgba_pixels(ctx, x, y, width, height, format, type, unpack, pixels);
       break;
    default:
-      _mesa_error( ctx, GL_INVALID_ENUM, "glDrawPixels(format)" );
+      _mesa_problem(ctx, "unexpected format in _swrast_DrawPixels");
       /* don't return yet, clean-up */
    }
 
@@ -978,9 +944,11 @@ _swrast_DrawDepthPixelsMESA( GLcontext *ctx,
    switch (colorFormat) {
    case GL_COLOR_INDEX:
       if (ctx->Visual.rgbMode)
-	 draw_rgba_pixels(ctx, x,y, width, height, colorFormat, colorType, unpack, colors);
+	 draw_rgba_pixels(ctx, x,y, width, height, colorFormat, colorType,
+                          unpack, colors);
       else
-	 draw_index_pixels(ctx, x, y, width, height, colorType, unpack, colors);
+	 draw_index_pixels(ctx, x, y, width, height, colorType,
+                           unpack, colors);
       break;
    case GL_RED:
    case GL_GREEN:
@@ -993,11 +961,11 @@ _swrast_DrawDepthPixelsMESA( GLcontext *ctx,
    case GL_RGBA:
    case GL_BGRA:
    case GL_ABGR_EXT:
-      draw_rgba_pixels(ctx, x, y, width, height, colorFormat, colorType, unpack, colors);
+      draw_rgba_pixels(ctx, x, y, width, height, colorFormat, colorType,
+                       unpack, colors);
       break;
    default:
-      _mesa_error( ctx, GL_INVALID_ENUM,
-                   "glDrawDepthPixelsMESA(colorFormat)" );
+      _mesa_problem(ctx, "unexpected format in glDrawDepthPixelsMESA");
    }
 
    RENDER_FINISH(swrast,ctx);
diff --git a/src/mesa/swrast/s_readpix.c b/src/mesa/swrast/s_readpix.c
index 6171b3440e..423661a872 100644
--- a/src/mesa/swrast/s_readpix.c
+++ b/src/mesa/swrast/s_readpix.c
@@ -40,7 +40,6 @@
 #include "s_stencil.h"
 
 
-
 /*
  * Read a block of color index pixels.
  */
@@ -54,23 +53,6 @@ read_index_pixels( GLcontext *ctx,
    struct gl_renderbuffer *rb = ctx->ReadBuffer->_ColorReadBuffer;
    GLint i;
 
-   /* error checking */
-   if (ctx->Visual.rgbMode) {
-      _mesa_error( ctx, GL_INVALID_OPERATION, "glReadPixels" );
-      return;
-   }
-
-   if (type != GL_BYTE &&
-       type != GL_UNSIGNED_BYTE &&
-       type != GL_SHORT &&
-       type != GL_UNSIGNED_SHORT &&
-       type != GL_INT &&
-       type != GL_UNSIGNED_INT &&
-       type != GL_FLOAT) {
-      _mesa_error( ctx, GL_INVALID_OPERATION, "glReadPixels(index type)");
-      return;
-   }
-
    if (!rb) {
       return;  /* no readbuffer OK */
    }
@@ -117,24 +99,6 @@ read_depth_pixels( GLcontext *ctx,
    /* width should never be > MAX_WIDTH since we did clipping earlier */
    ASSERT(width <= MAX_WIDTH);
 
-   /* Error checking */
-   if (fb->Visual.depthBits <= 0) {
-      /* No depth buffer */
-      _mesa_error( ctx, GL_INVALID_OPERATION, "glReadPixels" );
-      return;
-   }
-
-   if (type != GL_BYTE &&
-       type != GL_UNSIGNED_BYTE &&
-       type != GL_SHORT &&
-       type != GL_UNSIGNED_SHORT &&
-       type != GL_INT &&
-       type != GL_UNSIGNED_INT &&
-       type != GL_FLOAT) {
-      _mesa_error( ctx, GL_INVALID_OPERATION, "glReadPixels(depth type)");
-      return;
-   }
-
    if (!rb) {
       return;  /* no readbuffer OK */
    }
@@ -198,24 +162,6 @@ read_stencil_pixels( GLcontext *ctx,
       return;
    }
 
-   if (type != GL_BYTE &&
-       type != GL_UNSIGNED_BYTE &&
-       type != GL_SHORT &&
-       type != GL_UNSIGNED_SHORT &&
-       type != GL_INT &&
-       type != GL_UNSIGNED_INT &&
-       type != GL_FLOAT &&
-       type != GL_BITMAP) {
-      _mesa_error( ctx, GL_INVALID_OPERATION, "glReadPixels(stencil type)");
-      return;
-   }
-
-   if (fb->Visual.stencilBits <= 0) {
-      /* No stencil buffer */
-      _mesa_error( ctx, GL_INVALID_OPERATION, "glReadPixels" );
-      return;
-   }
-
    /* width should never be > MAX_WIDTH since we did clipping earlier */
    ASSERT(width <= MAX_WIDTH);
 
@@ -280,12 +226,13 @@ read_fast_rgba_pixels( GLcontext *ctx,
        * skip "skipRows" rows and skip "skipPixels" pixels/row.
        */
 #if CHAN_BITS == 8
-      if (format == GL_RGBA && type == GL_UNSIGNED_BYTE) {
+      if (format == GL_RGBA && type == GL_UNSIGNED_BYTE)
 #elif CHAN_BITS == 16
-      if (format == GL_RGBA && type == GL_UNSIGNED_SHORT) {
+      if (format == GL_RGBA && type == GL_UNSIGNED_SHORT)
 #else
-      if (0) {
+      if (0)
 #endif
+      {
          GLchan *dest = (GLchan *) pixels
                       + (skipRows * rowLength + skipPixels) * 4;
          GLint row;
@@ -331,46 +278,6 @@ read_rgba_pixels( GLcontext *ctx,
       return;
    }
 
-   /* do error checking on pixel type, format was already checked by caller */
-   switch (type) {
-      case GL_UNSIGNED_BYTE:
-      case GL_BYTE:
-      case GL_UNSIGNED_SHORT:
-      case GL_SHORT:
-      case GL_UNSIGNED_INT:
-      case GL_INT:
-      case GL_FLOAT:
-      case GL_UNSIGNED_BYTE_3_3_2:
-      case GL_UNSIGNED_BYTE_2_3_3_REV:
-      case GL_UNSIGNED_SHORT_5_6_5:
-      case GL_UNSIGNED_SHORT_5_6_5_REV:
-      case GL_UNSIGNED_SHORT_4_4_4_4:
-      case GL_UNSIGNED_SHORT_4_4_4_4_REV:
-      case GL_UNSIGNED_SHORT_5_5_5_1:
-      case GL_UNSIGNED_SHORT_1_5_5_5_REV:
-      case GL_UNSIGNED_INT_8_8_8_8:
-      case GL_UNSIGNED_INT_8_8_8_8_REV:
-      case GL_UNSIGNED_INT_10_10_10_2:
-      case GL_UNSIGNED_INT_2_10_10_10_REV:
-         /* valid pixel type */
-         break;
-      case GL_HALF_FLOAT_ARB:
-         if (!ctx->Extensions.ARB_half_float_pixel) {
-            _mesa_error( ctx, GL_INVALID_ENUM, "glReadPixels(type)" );
-            return;
-         }
-         break;
-      default:
-         _mesa_error( ctx, GL_INVALID_ENUM, "glReadPixels(type)" );
-         return;
-   }
-
-   if (!_mesa_is_legal_format_and_type(ctx, format, type) ||
-       format == GL_INTENSITY) {
-      _mesa_error(ctx, GL_INVALID_OPERATION, "glReadPixels(format or type)");
-      return;
-   }
-
    /* Try optimized path first */
    if (read_fast_rgba_pixels( ctx, x, y, width, height,
                               format, type, pixels, packing )) {
@@ -490,7 +397,7 @@ read_rgba_pixels( GLcontext *ctx,
 
 /**
  * Software fallback routine for ctx->Driver.ReadPixels().
- * We wind up using the swrast->ReadSpan() routines to do the job.
+ * By time we get here, all error checking will have been done.
  */
 void
 _swrast_ReadPixels( GLcontext *ctx,
@@ -567,7 +474,7 @@ _swrast_ReadPixels( GLcontext *ctx,
                           format, type, pixels, &clippedPacking);
 	 break;
       default:
-	 _mesa_error( ctx, GL_INVALID_ENUM, "glReadPixels(format)" );
+	 _mesa_problem(ctx, "unexpected format in _swrast_ReadPixels");
          /* don't return yet, clean-up */
    }