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

[Tapani Pälli]

On 11/18/2015 01:31 AM, Fredrik Höglund wrote:
> On Tuesday 17 November 2015, Tapani Pälli wrote:
>> On 11/16/2015 08:55 AM, Tapani Pälli wrote:
>>>
>>> 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.
>> Sorry, the question does not make sense. What I meant is that do you
>> know some app that would fail this test to help debugging/fixing the issue?
> No, but the following example should trigger the problem:
>
> /* Enable arrays 0 and 1 */
> glEnableVertexAttribArray(0);
> glEnableVertexAttribArray(1);
>
> /* Make both arrays use VBO binding point #0 */
> glVertexAttribBinding(0, 0);
> glVertexAttribBinding(1, 0);
>
> /* Bind a buffer object to VBO binding point #0 */
> glBindVertexBuffer(0, ...);
>
> /* Bind a draw indirect buffer */
> glBindBuffer(GL_DRAW_INDIRECT_BUFFER, ...);
>
> /* This call will now generate an INVALID_OPERATION error since
>   * no buffer is bound to VBO binding point #1, even though none
>   * of the enabled arrays use it.
>   */
> glDrawArraysIndirect(...);

Thanks, I'll try to reproduce with a test and fix this issue.

>>>>> +      _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;
>>>>>       }
>>>>>    }
>>>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev