On 02/02/2012 10:26 AM, Brian Paul wrote:
[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.

That's right!  That's issue #4 in the GL_ARB_texture_rgb10_a2ui spec:

    4. It is possible to load packed 10_10_10_2 unsigned integer data
       into GL via TexImage without this extension?

       RESOLVED:  No.  The EXT_texture_integer extension, as later
       incorporated into OpenGL 3.0, added new integer pixel format
       enums (e.g., RGBA_INTEGER) and texture formats (e.g.,
       RGBA16UI). All texture formats added by that extension had a
       "matching" non-packed format and type combination, so there
       wasn't a strong need to explicitly support packed pixel
       types for integer pixel formats.

       The texture format added by this extension logically maps to
       a "packed" format/type combination, so we add this support
       by adding RGB_INTEGER, RGBA_INTEGER, and BGRA_INTEGER (as
       appropriate) to the list of formats supported by packed pixel
       data types.

       Even though we are only adding one "packed" texture format,
       we chose to allow all packed types with corresponding integer
       formats for orthogonality.

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.

'grep -r ARB_texture_rgb10_a2ui src/' indicates not.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to