[Mesa-dev] [PATCH] mesa: check glBegin()/glDrawArrays()/etc mode with _mesa_valid_prim_mode()

[Yuanhan Liu]

On Tue, Sep 20, 2011 at 08:51:45AM -0600, Brian Paul wrote:
> From: Brian Paul <brianp at vmware.com>
> 
> We now raise an GL_INVALID_ENUM in glBegin() if mode is illegal, as was
> done in Yuanhan Liu's original patch.
> 
> Take geometry shaders support into account too.
> ---
>  src/mesa/main/api_validate.c |   31 ++++++++++++++++++++++++++-----
>  src/mesa/main/api_validate.h |    5 +++++
>  src/mesa/main/dlist.c        |    5 +++--
>  src/mesa/vbo/vbo_exec_api.c  |    7 +++++++
>  4 files changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
> index 699b414..1fcf5cd 100644
> --- a/src/mesa/main/api_validate.c
> +++ b/src/mesa/main/api_validate.c
> @@ -199,6 +199,27 @@ check_index_bounds(struct gl_context *ctx, GLsizei count, GLenum type,
>  
>  
>  /**
> + * Is 'mode' a valid value for glBegin(), glDrawArrays(), glDrawElements(),
> + * etc?  The set of legal values depends on whether geometry shaders/programs
> + * are supported.
> + */
> +GLboolean
> +_mesa_valid_prim_mode(const struct gl_context *ctx, GLenum mode)
> +{
> +   if (ctx->Extensions.ARB_geometry_shader4 &&
> +       mode > GL_TRIANGLE_STRIP_ADJACENCY_ARB) {
> +      return GL_FALSE;
> +   }
> +   else if (mode > GL_POLYGON) {
> +      return GL_FALSE;
> +   }
> +   else {
> +      return GL_TRUE;
> +   }
> +}
> +
> +
> +/**
>   * Error checking for glDrawElements().  Includes parameter checking
>   * and VBO bounds checking.
>   * \return GL_TRUE if OK to render, GL_FALSE if error found
> @@ -216,7 +237,7 @@ _mesa_validate_DrawElements(struct gl_context *ctx,
>        return GL_FALSE;
>     }
>  
> -   if (mode > GL_TRIANGLE_STRIP_ADJACENCY_ARB) {
> +   if (!_mesa_valid_prim_mode(ctx, mode)) {
>        _mesa_error(ctx, GL_INVALID_ENUM, "glDrawElements(mode)" );
>        return GL_FALSE;
>     }
> @@ -273,7 +294,7 @@ _mesa_validate_DrawRangeElements(struct gl_context *ctx, GLenum mode,
>        return GL_FALSE;
>     }
>  
> -   if (mode > GL_TRIANGLE_STRIP_ADJACENCY_ARB) {
> +   if (!_mesa_valid_prim_mode(ctx, mode)) {
>        _mesa_error(ctx, GL_INVALID_ENUM, "glDrawRangeElements(mode)" );
>        return GL_FALSE;
>     }
> @@ -332,7 +353,7 @@ _mesa_validate_DrawArrays(struct gl_context *ctx,
>        return GL_FALSE;
>     }
>  
> -   if (mode > GL_TRIANGLE_STRIP_ADJACENCY_ARB) {
> +   if (!_mesa_valid_prim_mode(ctx, mode)) {
>        _mesa_error(ctx, GL_INVALID_ENUM, "glDrawArrays(mode)" );
>        return GL_FALSE;
>     }
> @@ -362,7 +383,7 @@ _mesa_validate_DrawArraysInstanced(struct gl_context *ctx, GLenum mode, GLint fi
>        return GL_FALSE;
>     }
>  
> -   if (mode > GL_TRIANGLE_STRIP_ADJACENCY_ARB) {
> +   if (!_mesa_valid_prim_mode(ctx, mode)) {
>        _mesa_error(ctx, GL_INVALID_ENUM,
>                    "glDrawArraysInstanced(mode=0x%x)", mode);
>        return GL_FALSE;
> @@ -408,7 +429,7 @@ _mesa_validate_DrawElementsInstanced(struct gl_context *ctx,
>        return GL_FALSE;
>     }
>  
> -   if (mode > GL_TRIANGLE_STRIP_ADJACENCY_ARB) {
> +   if (!_mesa_valid_prim_mode(ctx, mode)) {
>        _mesa_error(ctx, GL_INVALID_ENUM,
>                    "glDrawElementsInstanced(mode = 0x%x)", mode);
>        return GL_FALSE;
> diff --git a/src/mesa/main/api_validate.h b/src/mesa/main/api_validate.h
> index 09e9522..7d6a660 100644
> --- a/src/mesa/main/api_validate.h
> +++ b/src/mesa/main/api_validate.h
> @@ -39,6 +39,11 @@ _mesa_max_buffer_index(struct gl_context *ctx, GLuint count, GLenum type,
>                         const void *indices,
>                         struct gl_buffer_object *elementBuf);
>  
> +
> +extern GLboolean
> +_mesa_valid_prim_mode(const struct gl_context *ctx, GLenum mode);
> +
> +
>  extern GLboolean
>  _mesa_validate_DrawArrays(struct gl_context *ctx,
>  			  GLenum mode, GLint start, GLsizei count);
> diff --git a/src/mesa/main/dlist.c b/src/mesa/main/dlist.c
> index 9bba521..f11dae9 100644
> --- a/src/mesa/main/dlist.c
> +++ b/src/mesa/main/dlist.c
> @@ -34,6 +34,7 @@
>  #include "api_arrayelt.h"
>  #include "api_exec.h"
>  #include "api_loopback.h"
> +#include "api_validate.h"
>  #if FEATURE_ATI_fragment_shader
>  #include "atifragshader.h"
>  #endif
> @@ -5762,8 +5763,8 @@ save_Begin(GLenum mode)
>     Node *n;
>     GLboolean error = GL_FALSE;
>  
> -   if ( /*mode < GL_POINTS || */ mode > GL_POLYGON) {
> -      _mesa_compile_error(ctx, GL_INVALID_ENUM, "Begin (mode)");
> +   if (!_mesa_valid_prim_mode(ctx, mode)) {
> +      _mesa_compile_error(ctx, GL_INVALID_ENUM, "glBegin(mode)");
>        error = GL_TRUE;
>     }
>     else if (ctx->Driver.CurrentSavePrimitive == PRIM_UNKNOWN) {
> diff --git a/src/mesa/vbo/vbo_exec_api.c b/src/mesa/vbo/vbo_exec_api.c
> index cad7c46..150589b 100644
> --- a/src/mesa/vbo/vbo_exec_api.c
> +++ b/src/mesa/vbo/vbo_exec_api.c
> @@ -42,6 +42,7 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
>  #include "main/light.h"
>  #include "main/api_arrayelt.h"
>  #include "main/api_noop.h"
> +#include "main/api_validate.h"
>  #include "main/dispatch.h"
>  
>  #include "vbo_context.h"
> @@ -552,6 +553,7 @@ static void GLAPIENTRY vbo_exec_EvalPoint2( GLint i, GLint j )
>  #endif /* FEATURE_evaluators */
>  
>  
> +
>  /**
>   * Called via glBegin.
>   */
> @@ -563,6 +565,11 @@ static void GLAPIENTRY vbo_exec_Begin( GLenum mode )
>        struct vbo_exec_context *exec = &vbo_context(ctx)->exec;
>        int i;
>  
> +      if (!_mesa_valid_prim_mode(ctx, mode)) {
> +         _mesa_error(ctx, GL_INVALID_ENUM, "glBegin");
> +         return;
> +      }
> +

Sorry, shouldn't we add the mode validation after the if(ctx->NewState)
code block to make sure the previous states are updated?


Thanks,
Yuanhan Liu

>        if (ctx->NewState) {
>  	 _mesa_update_state( ctx );
>  
> -- 
> 1.7.3.4
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev