[Mesa-dev] [PATCH] mesa: error out in indirect draw when vertex bindings mismatch
Samuel Iglesias Gonsálvez
siglesias at igalia.com
Mon Nov 16 23:14:40 PST 2015
On 17/11/15 07:38, Tapani Pälli wrote:
>
>
> On 11/16/2015 03:12 PM, Samuel Iglesias Gonsálvez wrote:
>>
>>
>> On 13/11/15 16:55, Tapani Pälli wrote:
>>> On 11/13/2015 03:40 PM, Samuel Iglesias Gonsálvez wrote:
>>>>
>>>> On 13/11/15 11:32, 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) {
>>>>> + _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);
>>>>> +
>>>> Should't it be VERT_BIT_GENERIC()?
>>>
>>> I used VERT_BIT because that is used when enabling vertex arrays and
>>> this mask should match that one.
>>>
>>
>> For that reason, I think it is VERT_BIT_GENERIC(). See:
>>
>> http://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/main/varray.c#n759
>>
>> Or am I missing something?
>
> In bind_vertex_buffer, 'index' includes already the offset added by
> VERT_BIT_GENERIC, if VERT_BIT_GENERIC were used, it would add offset
> twice and mask would not match, using VERT_BIT makes exact match because
> of the offset.
>
OK, you are right. This patch is:
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
Thanks!
Sam
>
>> Sam
>>
>>>> Sam
>>>>
>>>>> vao->NewArrays |= binding->_BoundArrays;
>>>>> }
>>>>> }
>>>>>
>>>
>>>
>
More information about the mesa-dev
mailing list