[Mesa-dev] [PATCH 2/3] mesa: Check that all buffers are bound in BeginTransformFeedback.

[Ian Romanick]

On 01/02/2012 09:37 PM, Paul Berry wrote:
>  From the EXT_transform_feedback spec:
>
>      The error INVALID_OPERATION is generated by
>      BeginTransformFeedbackEXT if any transform feedback buffer object
>      binding point used in transform feedback mode does not have a
>      buffer object bound.
>
> This required adding a new NumBuffers field to the
> gl_transform_feedback_info struct, to keep track of how many transform
> feedback buffers are required by the current program.
>
> Fixes Piglit tests:
> - EXT_transform_feedback/api-errors interleaved_unbound
> - EXT_transform_feedback/api-errors separate_unbound_0_1
> - EXT_transform_feedback/api-errors separate_unbound_0_2
> - EXT_transform_feedback/api-errors separate_unbound_1_2
> ---
>   src/glsl/linker.cpp               |    7 +++++--
>   src/mesa/main/mtypes.h            |    5 +++++
>   src/mesa/main/transformfeedback.c |   12 ++++++++++++
>   3 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index ed9a5d7..43be205 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -1889,11 +1889,14 @@ store_tfeedback_info(struct gl_context *ctx, struct gl_shader_program *prog,
>                        tfeedback_decl *tfeedback_decls)
>   {
>      unsigned total_tfeedback_components = 0;
> +   bool separate_attribs_mode =
> +      prog->TransformFeedback.BufferMode == GL_SEPARATE_ATTRIBS;
>      memset(&prog->LinkedTransformFeedback, 0,
>             sizeof(prog->LinkedTransformFeedback));
> +   prog->LinkedTransformFeedback.NumBuffers =
> +      separate_attribs_mode ? num_tfeedback_decls : 1;
>      for (unsigned i = 0; i<  num_tfeedback_decls; ++i) {
> -      unsigned buffer =
> -         prog->TransformFeedback.BufferMode == GL_SEPARATE_ATTRIBS ? i : 0;
> +      unsigned buffer = separate_attribs_mode ? i : 0;

Is this actually necessary?  If separate_attribs_mode is false, 
num_tfeedback_decls is 1, and i can only ever be 0.  Right?

>         if (!tfeedback_decls[i].store(prog,&prog->LinkedTransformFeedback,
>                                       buffer))
>            return false;
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index d520f98..456131f 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -1821,6 +1821,11 @@ struct gl_uniform_list;
>   struct gl_transform_feedback_info {
>      unsigned NumOutputs;
>
> +   /**
> +    * Number of transform feedback buffers in use by this program.
> +    */
> +   unsigned NumBuffers;
> +
>      struct {
>         unsigned OutputRegister;
>         unsigned OutputBuffer;
> diff --git a/src/mesa/main/transformfeedback.c b/src/mesa/main/transformfeedback.c
> index 305589d..6e93b3b 100644
> --- a/src/mesa/main/transformfeedback.c
> +++ b/src/mesa/main/transformfeedback.c
> @@ -342,9 +342,12 @@ void GLAPIENTRY
>   _mesa_BeginTransformFeedback(GLenum mode)
>   {
>      struct gl_transform_feedback_object *obj;
> +   struct gl_transform_feedback_info *info;
> +   int i;
>      GET_CURRENT_CONTEXT(ctx);
>
>      obj = ctx->TransformFeedback.CurrentObject;
> +   info =&ctx->Shader.CurrentVertexProgram->LinkedTransformFeedback;
>
>      switch (mode) {
>      case GL_POINTS:
> @@ -363,6 +366,15 @@ _mesa_BeginTransformFeedback(GLenum mode)
>         return;
>      }
>
> +   for (i = 0; i<  info->NumBuffers; ++i) {
> +      if (obj->BufferNames[i] == 0) {
> +         _mesa_error(ctx, GL_INVALID_OPERATION,
> +                     "glBeginTransformFeedback(binding point %d does not have "
> +                     "a buffer object bound)", i);
> +         return;
> +      }
> +   }
> +
>      FLUSH_VERTICES(ctx, _NEW_TRANSFORM_FEEDBACK);
>      obj->Active = GL_TRUE;
>      ctx->TransformFeedback.Mode = mode;