From 627b435dfe17698a1c69e9a259838fc6f2e6bd4e Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Tue, 7 Feb 2012 07:42:33 -0700 Subject: [PATCH] mesa: new _mesa_error_check_format_and_type() function This replaces the _mesa_is_legal_format_and_type() function. According to the spec, some invalid format/type combinations to glDrawPixels, ReadPixels and glTexImage should generate GL_INVALID_ENUM but others should generate GL_INVALID_OPERATION. With the old function we didn't make that distinction and generated GL_INVALID_ENUM errors instead of GL_INVALID_OPERATION. The new function returns one of those errors or GL_NO_ERROR. This will also let us remove some redundant format/type checks in follow-on commit. v2: add more checks for ARB_texture_rgb10_a2ui at the top of _mesa_error_check_format_and_type() per Ian. Signed-off-by: Brian Paul --- src/mesa/main/image.c | 216 +++++++++++++++++++++++++++++++------------- src/mesa/main/image.h | 6 +- src/mesa/main/readpix.c | 9 +- src/mesa/main/texgetimage.c | 16 ++-- src/mesa/main/teximage.c | 25 ++--- 5 files changed, 178 insertions(+), 94 deletions(-) diff --git a/src/mesa/main/image.c b/src/mesa/main/image.c index 3491704..750db94 100644 --- a/src/mesa/main/image.c +++ b/src/mesa/main/image.c @@ -356,18 +356,83 @@ _mesa_bytes_per_pixel( GLenum format, GLenum type ) /** - * Test for a legal pixel format and type. + * Do error checking of format/type combinations for glReadPixels, + * glDrawPixels and glTex[Sub]Image. Note that depending on the format + * and type values, we may either generate GL_INVALID_OPERATION or + * GL_INVALID_ENUM. * * \param format pixel format. * \param type pixel type. * - * \return GL_TRUE if the given pixel format and type are legal, or GL_FALSE - * otherwise. + * \return GL_INVALID_ENUM, GL_INVALID_OPERATION or GL_NO_ERROR */ -GLboolean -_mesa_is_legal_format_and_type(const struct gl_context *ctx, - GLenum format, GLenum type) +GLenum +_mesa_error_check_format_and_type(const struct gl_context *ctx, + GLenum format, GLenum type) { + /* special type-based checks (see glReadPixels, glDrawPixels error lists) */ + switch (type) { + case GL_BITMAP: + if (format != GL_COLOR_INDEX && format != GL_STENCIL_INDEX) { + return GL_INVALID_ENUM; + } + break; + + 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: + if (format == GL_RGB) { + break; /* OK */ + } + if (format == GL_RGB_INTEGER_EXT && + ctx->Extensions.ARB_texture_rgb10_a2ui) { + break; /* OK */ + } + return GL_INVALID_OPERATION; + + 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: + if (format == GL_RGBA || + format == GL_BGRA || + format == GL_ABGR_EXT) { + break; /* OK */ + } + if ((format == GL_RGBA_INTEGER_EXT || format == GL_BGRA_INTEGER_EXT) && + ctx->Extensions.ARB_texture_rgb10_a2ui) { + break; /* OK */ + } + return GL_INVALID_OPERATION; + + case GL_UNSIGNED_INT_24_8: + if (!ctx->Extensions.EXT_packed_depth_stencil) { + return GL_INVALID_ENUM; + } + if (format != GL_DEPTH_STENCIL) { + return GL_INVALID_OPERATION; + } + return GL_NO_ERROR; + + case GL_FLOAT_32_UNSIGNED_INT_24_8_REV: + if (!ctx->Extensions.ARB_depth_buffer_float) { + return GL_INVALID_ENUM; + } + if (format != GL_DEPTH_STENCIL) { + return GL_INVALID_OPERATION; + } + return GL_NO_ERROR; + + default: + ; /* fall-through */ + } + + /* now, for each format, check the type for compatibility */ switch (format) { case GL_COLOR_INDEX: case GL_STENCIL_INDEX: @@ -380,12 +445,14 @@ _mesa_is_legal_format_and_type(const struct gl_context *ctx, case GL_INT: case GL_UNSIGNED_INT: case GL_FLOAT: - return GL_TRUE; - case GL_HALF_FLOAT_ARB: - return ctx->Extensions.ARB_half_float_pixel; + return GL_NO_ERROR; + case GL_HALF_FLOAT: + return ctx->Extensions.ARB_half_float_pixel + ? GL_NO_ERROR : GL_INVALID_ENUM; default: - return GL_FALSE; + return GL_INVALID_ENUM; } + case GL_RED: case GL_GREEN: case GL_BLUE: @@ -404,16 +471,17 @@ _mesa_is_legal_format_and_type(const struct gl_context *ctx, case GL_INT: case GL_UNSIGNED_INT: case GL_FLOAT: - return GL_TRUE; - case GL_HALF_FLOAT_ARB: - return ctx->Extensions.ARB_half_float_pixel; + return GL_NO_ERROR; + case GL_HALF_FLOAT: + return ctx->Extensions.ARB_half_float_pixel + ? GL_NO_ERROR : GL_INVALID_ENUM; default: - return GL_FALSE; + return GL_INVALID_ENUM; } + case GL_RG: if (!ctx->Extensions.ARB_texture_rg) - return GL_FALSE; - + return GL_INVALID_ENUM; switch (type) { case GL_BYTE: case GL_UNSIGNED_BYTE: @@ -422,12 +490,14 @@ _mesa_is_legal_format_and_type(const struct gl_context *ctx, case GL_INT: case GL_UNSIGNED_INT: case GL_FLOAT: - return GL_TRUE; - case GL_HALF_FLOAT_ARB: - return ctx->Extensions.ARB_half_float_pixel; + return GL_NO_ERROR; + case GL_HALF_FLOAT: + return ctx->Extensions.ARB_half_float_pixel + ? GL_NO_ERROR : GL_INVALID_ENUM; default: - return GL_FALSE; + return GL_INVALID_ENUM; } + case GL_RGB: switch (type) { case GL_BYTE: @@ -441,16 +511,20 @@ _mesa_is_legal_format_and_type(const struct gl_context *ctx, case GL_UNSIGNED_BYTE_2_3_3_REV: case GL_UNSIGNED_SHORT_5_6_5: case GL_UNSIGNED_SHORT_5_6_5_REV: - return GL_TRUE; - case GL_HALF_FLOAT_ARB: - return ctx->Extensions.ARB_half_float_pixel; + return GL_NO_ERROR; + case GL_HALF_FLOAT: + return ctx->Extensions.ARB_half_float_pixel + ? GL_NO_ERROR : GL_INVALID_ENUM; case GL_UNSIGNED_INT_5_9_9_9_REV: - return ctx->Extensions.EXT_texture_shared_exponent; + return ctx->Extensions.EXT_texture_shared_exponent + ? GL_NO_ERROR : GL_INVALID_ENUM; case GL_UNSIGNED_INT_10F_11F_11F_REV: - return ctx->Extensions.EXT_packed_float; + return ctx->Extensions.EXT_packed_float + ? GL_NO_ERROR : GL_INVALID_ENUM; default: - return GL_FALSE; + return GL_INVALID_ENUM; } + case GL_BGR: switch (type) { /* NOTE: no packed types are supported with BGR. That's @@ -463,12 +537,14 @@ _mesa_is_legal_format_and_type(const struct gl_context *ctx, case GL_INT: case GL_UNSIGNED_INT: case GL_FLOAT: - return GL_TRUE; - case GL_HALF_FLOAT_ARB: - return ctx->Extensions.ARB_half_float_pixel; + return GL_NO_ERROR; + case GL_HALF_FLOAT: + return ctx->Extensions.ARB_half_float_pixel + ? GL_NO_ERROR : GL_INVALID_ENUM; default: - return GL_FALSE; + return GL_INVALID_ENUM; } + case GL_RGBA: case GL_BGRA: case GL_ABGR_EXT: @@ -488,28 +564,37 @@ _mesa_is_legal_format_and_type(const struct gl_context *ctx, 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: - return GL_TRUE; - case GL_HALF_FLOAT_ARB: - return ctx->Extensions.ARB_half_float_pixel; + return GL_NO_ERROR; + case GL_HALF_FLOAT: + return ctx->Extensions.ARB_half_float_pixel + ? GL_NO_ERROR : GL_INVALID_ENUM; default: - return GL_FALSE; + return GL_INVALID_ENUM; } + case GL_YCBCR_MESA: + if (!ctx->Extensions.MESA_ycbcr_texture) + return GL_INVALID_ENUM; if (type == GL_UNSIGNED_SHORT_8_8_MESA || type == GL_UNSIGNED_SHORT_8_8_REV_MESA) - return GL_TRUE; + return GL_NO_ERROR; else - return GL_FALSE; + return GL_INVALID_OPERATION; + case GL_DEPTH_STENCIL_EXT: - if ((ctx->Extensions.EXT_packed_depth_stencil && - type == GL_UNSIGNED_INT_24_8_EXT) || - (ctx->Extensions.ARB_depth_buffer_float && - type == GL_FLOAT_32_UNSIGNED_INT_24_8_REV)) - return GL_TRUE; + if (ctx->Extensions.EXT_packed_depth_stencil && + type == GL_UNSIGNED_INT_24_8) + return GL_NO_ERROR; + else if (ctx->Extensions.ARB_depth_buffer_float && + type == GL_FLOAT_32_UNSIGNED_INT_24_8_REV) + return GL_NO_ERROR; else - return GL_FALSE; + return GL_INVALID_ENUM; + case GL_DUDV_ATI: case GL_DU8DV8_ATI: + if (!ctx->Extensions.ATI_envmap_bumpmap) + return GL_INVALID_ENUM; switch (type) { case GL_BYTE: case GL_UNSIGNED_BYTE: @@ -518,9 +603,9 @@ _mesa_is_legal_format_and_type(const struct gl_context *ctx, case GL_INT: case GL_UNSIGNED_INT: case GL_FLOAT: - return GL_TRUE; + return GL_NO_ERROR; default: - return GL_FALSE; + return GL_INVALID_ENUM; } /* integer-valued formats */ @@ -536,10 +621,11 @@ _mesa_is_legal_format_and_type(const struct gl_context *ctx, case GL_UNSIGNED_SHORT: case GL_INT: case GL_UNSIGNED_INT: - return ctx->VersionMajor >= 3 || - ctx->Extensions.EXT_texture_integer; + return (ctx->VersionMajor >= 3 || + ctx->Extensions.EXT_texture_integer) + ? GL_NO_ERROR : GL_INVALID_ENUM; default: - return GL_FALSE; + return GL_INVALID_ENUM; } case GL_RGB_INTEGER_EXT: @@ -550,15 +636,17 @@ _mesa_is_legal_format_and_type(const struct gl_context *ctx, case GL_UNSIGNED_SHORT: case GL_INT: case GL_UNSIGNED_INT: - return ctx->VersionMajor >= 3 || - ctx->Extensions.EXT_texture_integer; + return (ctx->VersionMajor >= 3 || + ctx->Extensions.EXT_texture_integer) + ? GL_NO_ERROR : GL_INVALID_ENUM; 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: - return ctx->Extensions.ARB_texture_rgb10_a2ui; + return ctx->Extensions.ARB_texture_rgb10_a2ui + ? GL_NO_ERROR : GL_INVALID_ENUM; default: - return GL_FALSE; + return GL_INVALID_ENUM; } case GL_BGR_INTEGER_EXT: @@ -570,10 +658,11 @@ _mesa_is_legal_format_and_type(const struct gl_context *ctx, case GL_INT: case GL_UNSIGNED_INT: /* NOTE: no packed formats w/ BGR format */ - return ctx->VersionMajor >= 3 || - ctx->Extensions.EXT_texture_integer; + return (ctx->VersionMajor >= 3 || + ctx->Extensions.EXT_texture_integer) + ? GL_NO_ERROR : GL_INVALID_ENUM; default: - return GL_FALSE; + return GL_INVALID_ENUM; } case GL_RGBA_INTEGER_EXT: @@ -585,8 +674,9 @@ _mesa_is_legal_format_and_type(const struct gl_context *ctx, case GL_UNSIGNED_SHORT: case GL_INT: case GL_UNSIGNED_INT: - return ctx->VersionMajor >= 3 || - ctx->Extensions.EXT_texture_integer; + return (ctx->VersionMajor >= 3 || + ctx->Extensions.EXT_texture_integer) + ? GL_NO_ERROR : GL_INVALID_ENUM; 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: @@ -595,9 +685,10 @@ _mesa_is_legal_format_and_type(const struct gl_context *ctx, 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: - return ctx->Extensions.ARB_texture_rgb10_a2ui; + return ctx->Extensions.ARB_texture_rgb10_a2ui + ? GL_NO_ERROR : GL_INVALID_ENUM; default: - return GL_FALSE; + return GL_INVALID_ENUM; } case GL_LUMINANCE_INTEGER_EXT: @@ -609,15 +700,16 @@ _mesa_is_legal_format_and_type(const struct gl_context *ctx, case GL_UNSIGNED_SHORT: case GL_INT: case GL_UNSIGNED_INT: - return ctx->Extensions.EXT_texture_integer; + return ctx->Extensions.EXT_texture_integer + ? GL_NO_ERROR : GL_INVALID_ENUM; default: - return GL_FALSE; + return GL_INVALID_ENUM; } default: - ; /* fall-through */ + return GL_INVALID_ENUM; } - return GL_FALSE; + return GL_NO_ERROR; } diff --git a/src/mesa/main/image.h b/src/mesa/main/image.h index e4961ed..f1ed883 100644 --- a/src/mesa/main/image.h +++ b/src/mesa/main/image.h @@ -53,9 +53,9 @@ _mesa_components_in_format( GLenum format ); extern GLint _mesa_bytes_per_pixel( GLenum format, GLenum type ); -extern GLboolean -_mesa_is_legal_format_and_type(const struct gl_context *ctx, - GLenum format, GLenum type); +extern GLenum +_mesa_error_check_format_and_type(const struct gl_context *ctx, + GLenum format, GLenum type); extern GLboolean _mesa_is_color_format(GLenum format); diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c index 908a55e..6c64cbe 100644 --- a/src/mesa/main/readpix.c +++ b/src/mesa/main/readpix.c @@ -588,6 +588,7 @@ _mesa_error_check_format_type(struct gl_context *ctx, GLenum format, { const char *readDraw = drawing ? "Draw" : "Read"; const GLboolean reading = !drawing; + GLenum err; /* state validation should have already been done */ ASSERT(ctx->NewState == 0x0); @@ -609,9 +610,9 @@ _mesa_error_check_format_type(struct gl_context *ctx, GLenum format, } /* 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); + err = _mesa_error_check_format_and_type(ctx, format, type); + if (err != GL_NO_ERROR) { + _mesa_error(ctx, err, "gl%sPixels(format or type)", readDraw); return GL_TRUE; } @@ -715,7 +716,7 @@ _mesa_error_check_format_type(struct gl_context *ctx, GLenum format, } break; default: - /* this should have been caught in _mesa_is_legal_format_type() */ + /* this should have been caught in _mesa_error_check_format_type() */ _mesa_problem(ctx, "unexpected format in _mesa_%sPixels", readDraw); return GL_TRUE; } diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c index 42495c8..58fed11 100644 --- a/src/mesa/main/texgetimage.c +++ b/src/mesa/main/texgetimage.c @@ -734,7 +734,7 @@ getteximage_error_check(struct gl_context *ctx, GLenum target, GLint level, struct gl_texture_image *texImage; const GLint maxLevels = _mesa_max_texture_levels(ctx, target); const GLuint dimensions = (target == GL_TEXTURE_3D) ? 3 : 2; - GLenum baseFormat; + GLenum baseFormat, err; if (maxLevels == 0) { _mesa_error(ctx, GL_INVALID_ENUM, "glGetTexImage(target=0x%x)", target); @@ -777,6 +777,12 @@ getteximage_error_check(struct gl_context *ctx, GLenum target, GLint level, if (!ctx->Extensions.ATI_envmap_bumpmap && _mesa_is_dudv_format(format)) { _mesa_error(ctx, GL_INVALID_ENUM, "glGetTexImage(format)"); + return; + } + + err = _mesa_error_check_format_and_type(ctx, format, type); + if (err != GL_NO_ERROR) { + _mesa_error(ctx, err, "glGetTexImage(format/type)"); return GL_TRUE; } @@ -787,14 +793,6 @@ getteximage_error_check(struct gl_context *ctx, GLenum target, GLint level, return GL_TRUE; } - if (!_mesa_is_legal_format_and_type(ctx, format, type)) { - /* GL_INVALID_OPERATION is generated by a format/type - * mismatch (see the 1.2 spec page 94, sec 3.6.4.) - */ - _mesa_error(ctx, GL_INVALID_OPERATION, "glGetTexImage(target)"); - return GL_TRUE; - } - texImage = _mesa_select_tex_image(ctx, texObj, target, level); if (!texImage) { /* non-existant texture image */ diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c index 018aca0..25da753 100644 --- a/src/mesa/main/teximage.c +++ b/src/mesa/main/teximage.c @@ -1511,6 +1511,7 @@ texture_error_check( struct gl_context *ctx, const GLboolean isProxy = target == proxyTarget; GLboolean sizeOK = GL_TRUE; GLboolean colorFormat; + GLenum err; /* Even though there are no color-index textures, we still have to support * uploading color-index data and remapping it to RGB via the @@ -1579,16 +1580,10 @@ texture_error_check( struct gl_context *ctx, } /* Check incoming image format and type */ - if (!_mesa_is_legal_format_and_type(ctx, format, type)) { - /* Normally, GL_INVALID_OPERATION is generated by a format/type - * mismatch (see the 1.2 spec page 94, sec 3.6.4.). But with the - * GL_EXT_texture_integer extension, some combinations should generate - * GL_INVALID_ENUM instead (grr!). - */ + err = _mesa_error_check_format_and_type(ctx, format, type); + if (err != GL_NO_ERROR) { if (!isProxy) { - GLenum error = _mesa_is_integer_format(format) - ? GL_INVALID_ENUM : GL_INVALID_OPERATION; - _mesa_error(ctx, error, + _mesa_error(ctx, err, "glTexImage%dD(incompatible format 0x%x, type 0x%x)", dimensions, format, type); } @@ -1737,6 +1732,8 @@ subtexture_error_check( struct gl_context *ctx, GLuint dimensions, GLint width, GLint height, GLint depth, GLenum format, GLenum type ) { + GLenum err; + /* Basic level check */ if (level < 0 || level >= MAX_TEXTURE_LEVELS) { _mesa_error(ctx, GL_INVALID_ENUM, "glTexSubImage2D(level=%d)", level); @@ -1760,13 +1757,9 @@ subtexture_error_check( struct gl_context *ctx, GLuint dimensions, return GL_TRUE; } - if (!_mesa_is_legal_format_and_type(ctx, format, type)) { - /* As with the glTexImage2D check above, the error code here - * depends on texture integer. - */ - GLenum error = _mesa_is_integer_format(format) - ? GL_INVALID_OPERATION : GL_INVALID_ENUM; - _mesa_error(ctx, error, + err = _mesa_error_check_format_and_type(ctx, format, type); + if (err != GL_NO_ERROR) { + _mesa_error(ctx, err, "glTexSubImage%dD(incompatible format 0x%x, type 0x%x)", dimensions, format, type); return GL_TRUE; -- 2.7.4