[Mesa-dev] [PATCH 1/3] mesa: new _mesa_error_check_format_and_type() function

[Brian Paul]

[don't know why the indention is messed up below, but anyway...]

On 02/02/2012 10:02 AM, Ian Romanick wrote:
> On 02/02/2012 07:14 AM, Brian Paul wrote:
>> 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.
>> ---
>> src/mesa/main/image.c | 212 ++++++++++++++++++++++++++++++-------------
>> 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, 174 insertions(+), 94 deletions(-)
>>
>> diff --git a/src/mesa/main/image.c b/src/mesa/main/image.c
>> index 3491704..cc44468 100644
>> --- a/src/mesa/main/image.c
>> +++ b/src/mesa/main/image.c
>> @@ -356,18 +356,79 @@ _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&&
>> + format != GL_RGB_INTEGER_EXT) {
>> + return GL_INVALID_OPERATION;
>
> I don't think the packed types can be used with *_INTEGER. At the very
> least, the code in pack.c can't handle it. Table 3.8 in the 3.0 spec
> only lists RGB, RGBA, BGRA, and DEPTH_STENCIL as valid formats for
> packed types. The EXT_abgr spec adds ABGR_EXT, of course.

It looks like packed integer formats are added in GL 3.3.  But they're 
also listed in GL_ARB_texture_rgb10_a2ui which is supposedly based on 
GL 3.2.  It's not clear to me if GL_ARB_texture_rgb10_a2ui is supposed 
to enable all the packed integer formats or just the 10/10/10/2 packed 
format.  Hmmm.

Anyway, if you look further down in that function you'll see 
finer-grained format/type checking which checks for 
ctx->Extensions.ARB_texture_rgb10_a2ui for those cases.  But I suppose 
we need more checks at the earlier point to generate 
GL_INVALID_OPERATION anyway.  I can do that.

I haven't looked if any drivers support GL_ARB_texture_rgb10_a2ui.

-Brian