From 99796464c5f0fdb463c31a0e99b0896089b8bd80 Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Tue, 27 Sep 2005 01:25:24 +0000 Subject: [PATCH] Lift all the format/type error checking out of the _swrast_Draw/Read/CopyPixels functions into the _mesa_Draw/Read/CopyPixels functions. --- src/mesa/main/drawpix.c | 115 ++++++++++++++++++++++++++++++++++++++++++-- src/mesa/swrast/s_copypix.c | 45 ++++++++--------- src/mesa/swrast/s_drawpix.c | 56 +++++---------------- src/mesa/swrast/s_readpix.c | 105 +++------------------------------------- 4 files changed, 148 insertions(+), 173 deletions(-) diff --git a/src/mesa/main/drawpix.c b/src/mesa/main/drawpix.c index 81a5bb0..3ced207 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 2b4e845..ed6a02e 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 (srcyPixel.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 91ac13b..d908ca4 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 6171b34..423661a 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 */ } -- 2.7.4