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

2015-11-17 Thread Fredrik Höglund 
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 
> >>> ---
> >>>   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(...);

> >
> >>> +  _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@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-11-17 Thread 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 
---
   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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-11-16 Thread Samuel Iglesias Gonsálvez 


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 
>>> ---
>>>   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?

Sam

>> Sam
>>
>>> vao->NewArrays |= binding->_BoundArrays;
>>>  }
>>>   }
>>>
> 
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-11-16 Thread Tapani Pälli 



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 
---
   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.




Sam


Sam


 vao->NewArrays |= binding->_BoundArrays;
  }
   }





___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-11-16 Thread Samuel Iglesias Gonsálvez 


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 
> ---
>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 

Thanks!

Sam

> 
>> Sam
>>
 Sam

>  vao->NewArrays |= binding->_BoundArrays;
>   }
>}
>
>>>
>>>
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-11-16 Thread Tapani Pälli 



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 
---
  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?






+  _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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-11-15 Thread 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 
---
  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;
 }
  }




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-11-13 Thread Tapani Pälli 

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 
---
  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.



Sam


vao->NewArrays |= binding->_BoundArrays;
 }
  }



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-11-13 Thread Fredrik Höglund 
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 
> ---
>  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.

> +  _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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-11-13 Thread Tapani Pälli 
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 
---
 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);
+
   vao->NewArrays |= binding->_BoundArrays;
}
 }
-- 
2.4.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-11-13 Thread Samuel Iglesias Gonsálvez 


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 
> ---
>  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()?

Sam

>vao->NewArrays |= binding->_BoundArrays;
> }
>  }
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev