[Mesa-dev] [PATCH] mesa: error out in indirect draw when vertex bindings mismatch

[Tapani Pälli]

On 11/13/2015 07:18 PM, Fredrik Höglund wrote:
> On Friday 13 November 2015, Tapani Pälli wrote:
>> Patch adds additional mask for tracking which vertex buffer bindings
>> are set. This array can be directly compared to which vertex arrays
>> are enabled and should match when drawing.
>>
>> Fixes following CTS tests:
>>
>>     ES31-CTS.draw_indirect.negative-noVBO-arrays
>>     ES31-CTS.draw_indirect.negative-noVBO-elements
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/mesa/main/api_validate.c | 13 +++++++++++++
>>   src/mesa/main/mtypes.h       |  3 +++
>>   src/mesa/main/varray.c       |  5 +++++
>>   3 files changed, 21 insertions(+)
>>
>> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
>> index a490189..e82e89a 100644
>> --- a/src/mesa/main/api_validate.c
>> +++ b/src/mesa/main/api_validate.c
>> @@ -710,6 +710,19 @@ valid_draw_indirect(struct gl_context *ctx,
>>         return GL_FALSE;
>>      }
>>
>> +   /* From OpenGL ES 3.1 spec. section 10.5:
>> +    *     "An INVALID_OPERATION error is generated if zero is bound to
>> +    *     VERTEX_ARRAY_BINDING, DRAW_INDIRECT_BUFFER or to any enabled
>> +    *     vertex array."
>> +    *
>> +    * Here we check that vertex buffer bindings match with enabled
>> +    * vertex arrays.
>> +    */
>> +   if (ctx->Array.VAO->_Enabled != ctx->Array.VAO->VertexBindingMask) {
>
> This test only works when the enabled vertex arrays are associated with
> their default vertex buffer binding points.

Could you open up this more, is there some existing test or app that 
would do this? Would be great for testing purposes, all the indirect 
draw rendering CTS tests pass with this change.


>> +      _mesa_error(ctx, GL_INVALID_OPERATION, "%s(No VBO bound)", name);
>> +      return GL_FALSE;
>> +   }
>> +
>>      if (!_mesa_valid_prim_mode(ctx, mode, name))
>>         return GL_FALSE;
>>
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> index 4efdf1e..6c6187f 100644
>> --- a/src/mesa/main/mtypes.h
>> +++ b/src/mesa/main/mtypes.h
>> @@ -1419,6 +1419,9 @@ struct gl_vertex_array_object
>>      /** Vertex buffer bindings */
>>      struct gl_vertex_buffer_binding VertexBinding[VERT_ATTRIB_MAX];
>>
>> +   /** Mask indicating which binding points are set. */
>> +   GLbitfield64 VertexBindingMask;
>> +
>>      /** Mask of VERT_BIT_* values indicating which arrays are enabled */
>>      GLbitfield64 _Enabled;
>>
>> diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
>> index 887d0c0..0a94c5a 100644
>> --- a/src/mesa/main/varray.c
>> +++ b/src/mesa/main/varray.c
>> @@ -174,6 +174,11 @@ bind_vertex_buffer(struct gl_context *ctx,
>>         binding->Offset = offset;
>>         binding->Stride = stride;
>>
>> +      if (vbo == ctx->Shared->NullBufferObj)
>> +         vao->VertexBindingMask &= ~VERT_BIT(index);
>> +      else
>> +         vao->VertexBindingMask |= VERT_BIT(index);
>> +
>>         vao->NewArrays |= binding->_BoundArrays;
>>      }
>>   }
>>
>