On 08/22/2012 11:36 PM, Kenneth Graunke wrote:
On 08/22/2012 07:26 PM, Ian Romanick wrote:
From: Ian Romanick <ian.d.roman...@intel.com>

This consolidates the tests and makes the emitted error message
consistent.

Signed-off-by: Ian Romanick <ian.d.roman...@intel.com>
---
  src/mesa/main/api_validate.c |   46 +++++++++++++++++++++---------------------
  1 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index eaf614b..e8c8333 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -262,6 +262,25 @@ _mesa_valid_prim_mode(struct gl_context *ctx, GLenum mode, 
const char *name)
     return GL_TRUE;
  }

+/**
+ * Verify that the element type is valid.
+ *
+ * Generates \c GL_INVALID_ENUM and returns \c false if it is not.
+ */
+static bool
+_mesa_valid_element_type(struct gl_context *ctx, GLenum type, const char *name)
+{

Could we call this _mesa_valid_draw_elements_type()?  When I first saw
this, I was thinking more broadly..."elements of what?", "only unsigned
elements are allowed?", and so on.

+   switch (type) {
+   case GL_UNSIGNED_BYTE:
+   case GL_UNSIGNED_SHORT:
+   case GL_UNSIGNED_INT:
+      return true;
+
+   default:
+      _mesa_error(ctx, GL_INVALID_ENUM, "%s(type = 0x%04x)", name, type);

Could we also please do:

       _mesa_error(ctx, GL_INVALID_ENUM, "%s(type = %s)", name,
                   _mesa_lookup_enum_by_nr(type));

May as well make it a bit friendlier, right?  These enums are
unambiguous so the string will be correct.

Maybe. This is a case when an app is passing in a value that could never be valid, so who knows what it passed. 0? 1234? 42? When a value that could sometimes be valid is rejected, I like to print the string. When a value that could never be valid is rejected (and may not even be an enum), the string may or may not be more friendly.

I don't have too strong of any opinion either way.

Otherwise, I like the idea.
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>

+      return false;
+   }
+}

  /**
   * Error checking for glDrawElements().  Includes parameter checking
@@ -286,13 +305,8 @@ _mesa_validate_DrawElements(struct gl_context *ctx,
        return GL_FALSE;
     }

-   if (type != GL_UNSIGNED_INT &&
-       type != GL_UNSIGNED_BYTE &&
-       type != GL_UNSIGNED_SHORT)
-   {
-      _mesa_error(ctx, GL_INVALID_ENUM, "glDrawElements(type)" );
+   if (!_mesa_valid_element_type(ctx, type, "glDrawElements"))
        return GL_FALSE;
-   }

     if (!check_valid_to_render(ctx, "glDrawElements"))
        return GL_FALSE;
@@ -348,13 +362,8 @@ _mesa_validate_MultiDrawElements(struct gl_context *ctx,
        return GL_FALSE;
     }

-   if (type != GL_UNSIGNED_INT &&
-       type != GL_UNSIGNED_BYTE &&
-       type != GL_UNSIGNED_SHORT)
-   {
-      _mesa_error(ctx, GL_INVALID_ENUM, "glMultiDrawElements(type)" );
+   if (!_mesa_valid_element_type(ctx, type, "glMultiDrawElements"))
        return GL_FALSE;
-   }

     if (!check_valid_to_render(ctx, "glMultiDrawElements"))
        return GL_FALSE;
@@ -419,12 +428,8 @@ _mesa_validate_DrawRangeElements(struct gl_context *ctx, 
GLenum mode,
        return GL_FALSE;
     }

-   if (type != GL_UNSIGNED_INT &&
-       type != GL_UNSIGNED_BYTE &&
-       type != GL_UNSIGNED_SHORT) {
-      _mesa_error(ctx, GL_INVALID_ENUM, "glDrawRangeElements(type)" );
+   if (!_mesa_valid_element_type(ctx, type, "glDrawRangeElements"))
        return GL_FALSE;
-   }

     if (!check_valid_to_render(ctx, "glDrawRangeElements"))
        return GL_FALSE;
@@ -548,13 +553,8 @@ _mesa_validate_DrawElementsInstanced(struct gl_context 
*ctx,
        return GL_FALSE;
     }

-   if (type != GL_UNSIGNED_INT &&
-       type != GL_UNSIGNED_BYTE &&
-       type != GL_UNSIGNED_SHORT) {
-      _mesa_error(ctx, GL_INVALID_ENUM,
-                  "glDrawElementsInstanced(type=0x%x)", type);
+   if (!_mesa_valid_element_type(ctx, type, "glDrawElementsInstanced"))
        return GL_FALSE;
-   }

     if (numInstances <= 0) {
        if (numInstances < 0)

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to