Re: [Mesa-dev] [PATCH] mesa: ignore VAO IDs equal to 0 in glDeleteVertexArrays
Hello,
Thanks for your reply.
Yes you are right.
The new code was added 7 days ago)
Regards,
Andrii.
On Tue, Sep 4, 2018 at 8:53 PM Marek Olšák wrote:
> On Mon, Sep 3, 2018 at 5:05 AM, andrey simiklit
> wrote:
> > Hello,
> >
> > I believe that this check already added inside _mesa_lookup_vao:
> >
> >>_mesa_lookup_vao(struct gl_context *ctx, GLuint id)
> >>{
> >> if (id == 0) {
> >> return NULL;
> >> } else {
> >> ...
> >>}
>
> Your Mesa code is outdated.
>
> Marek
>
> >
> > I guess that the '_mesa_lookup_vao' function returns NULL for zero
> indexes.
> > So we will never pass into the 'if(obj)' for 0 index case.
> > So I think that this additional check which was added by this patch is
> > unnecessary here
> > because this functionality works fine for zero indexes as is.
> > Please correct me if am wrong.
> >
> > Regards,
> > Andrii.
> >
> > On Fri, Aug 31, 2018 at 10:24 AM Ian Romanick
> wrote:
> >>
> >> On 08/30/2018 07:42 PM, Marek Olšák wrote:
> >> > Sadly, there are no tests for this.
> >>
> >> Ok... I just sent one to the piglit list. It should be easy enough to
> >> create a few more for other glDeleteFoo functions. I'll save that for
> >> during boring meetings. ;)
> >>
> >> > Marek
> >> >
> >> > On Thu, Aug 30, 2018 at 6:24 PM, Ian Romanick
> >> > wrote:
> >> >> This patch is
> >> >>
> >> >> Reviewed-by: Ian Romanick
> >> >>
> >> >> Is there a piglit test? I wonder how many other glDeleteFoo
> functions
> >> >> mishandle the id=0 case. :(
> >> >>
> >> >> On 08/30/2018 12:16 PM, Marek Olšák wrote:
> >> >>> From: Marek Olšák
> >> >>>
> >> >>> This fixes a firefox crash.
> >> >>>
> >> >>> Fixes: 781a78914c798dc64005b37c6ca1224ce06803fc
> >> >>> ---
> >> >>> src/mesa/main/arrayobj.c | 4
> >> >>> 1 file changed, 4 insertions(+)
> >> >>>
> >> >>> diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
> >> >>> index a23031fe182..6e0665c0e5d 100644
> >> >>> --- a/src/mesa/main/arrayobj.c
> >> >>> +++ b/src/mesa/main/arrayobj.c
> >> >>> @@ -1007,20 +1007,24 @@ _mesa_BindVertexArray(GLuint id)
> >> >>> *
> >> >>> * \param n Number of array objects to delete.
> >> >>> * \param idsArray of \c n array object IDs.
> >> >>> */
> >> >>> static void
> >> >>> delete_vertex_arrays(struct gl_context *ctx, GLsizei n, const
> GLuint
> >> >>> *ids)
> >> >>> {
> >> >>> GLsizei i;
> >> >>>
> >> >>> for (i = 0; i < n; i++) {
> >> >>> + /* IDs equal to 0 should be silently ignored. */
> >> >>> + if (!ids[i])
> >> >>> + continue;
> >> >>> +
> >> >>>struct gl_vertex_array_object *obj = _mesa_lookup_vao(ctx,
> >> >>> ids[i]);
> >> >>>
> >> >>>if (obj) {
> >> >>> assert(obj->Name == ids[i]);
> >> >>>
> >> >>> /* If the array object is currently bound, the spec says
> >> >>> "the binding
> >> >>>* for that object reverts to zero and the default vertex
> >> >>> array
> >> >>>* becomes current."
> >> >>>*/
> >> >>> if (obj == ctx->Array.VAO)
> >> ___
> >> mesa-dev mailing list
> >> mesa-dev@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: ignore VAO IDs equal to 0 in glDeleteVertexArrays
On Mon, Sep 3, 2018 at 5:05 AM, andrey simiklit
wrote:
> Hello,
>
> I believe that this check already added inside _mesa_lookup_vao:
>
>>_mesa_lookup_vao(struct gl_context *ctx, GLuint id)
>>{
>> if (id == 0) {
>> return NULL;
>> } else {
>> ...
>>}
Your Mesa code is outdated.
Marek
>
> I guess that the '_mesa_lookup_vao' function returns NULL for zero indexes.
> So we will never pass into the 'if(obj)' for 0 index case.
> So I think that this additional check which was added by this patch is
> unnecessary here
> because this functionality works fine for zero indexes as is.
> Please correct me if am wrong.
>
> Regards,
> Andrii.
>
> On Fri, Aug 31, 2018 at 10:24 AM Ian Romanick wrote:
>>
>> On 08/30/2018 07:42 PM, Marek Olšák wrote:
>> > Sadly, there are no tests for this.
>>
>> Ok... I just sent one to the piglit list. It should be easy enough to
>> create a few more for other glDeleteFoo functions. I'll save that for
>> during boring meetings. ;)
>>
>> > Marek
>> >
>> > On Thu, Aug 30, 2018 at 6:24 PM, Ian Romanick
>> > wrote:
>> >> This patch is
>> >>
>> >> Reviewed-by: Ian Romanick
>> >>
>> >> Is there a piglit test? I wonder how many other glDeleteFoo functions
>> >> mishandle the id=0 case. :(
>> >>
>> >> On 08/30/2018 12:16 PM, Marek Olšák wrote:
>> >>> From: Marek Olšák
>> >>>
>> >>> This fixes a firefox crash.
>> >>>
>> >>> Fixes: 781a78914c798dc64005b37c6ca1224ce06803fc
>> >>> ---
>> >>> src/mesa/main/arrayobj.c | 4
>> >>> 1 file changed, 4 insertions(+)
>> >>>
>> >>> diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
>> >>> index a23031fe182..6e0665c0e5d 100644
>> >>> --- a/src/mesa/main/arrayobj.c
>> >>> +++ b/src/mesa/main/arrayobj.c
>> >>> @@ -1007,20 +1007,24 @@ _mesa_BindVertexArray(GLuint id)
>> >>> *
>> >>> * \param n Number of array objects to delete.
>> >>> * \param idsArray of \c n array object IDs.
>> >>> */
>> >>> static void
>> >>> delete_vertex_arrays(struct gl_context *ctx, GLsizei n, const GLuint
>> >>> *ids)
>> >>> {
>> >>> GLsizei i;
>> >>>
>> >>> for (i = 0; i < n; i++) {
>> >>> + /* IDs equal to 0 should be silently ignored. */
>> >>> + if (!ids[i])
>> >>> + continue;
>> >>> +
>> >>>struct gl_vertex_array_object *obj = _mesa_lookup_vao(ctx,
>> >>> ids[i]);
>> >>>
>> >>>if (obj) {
>> >>> assert(obj->Name == ids[i]);
>> >>>
>> >>> /* If the array object is currently bound, the spec says
>> >>> "the binding
>> >>>* for that object reverts to zero and the default vertex
>> >>> array
>> >>>* becomes current."
>> >>>*/
>> >>> if (obj == ctx->Array.VAO)
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: ignore VAO IDs equal to 0 in glDeleteVertexArrays
Hello,
I believe that this check already added inside _mesa_lookup_vao:
>_mesa_lookup_vao(struct gl_context *ctx, GLuint id)
>{
> if (id == 0) {
> return NULL;
> } else {
> ...
>}
I guess that the '_mesa_lookup_vao' function returns NULL for zero indexes.
So we will never pass into the 'if(obj)' for 0 index case.
So I think that this additional check which was added by this patch is
unnecessary here
because this functionality works fine for zero indexes as is.
Please correct me if am wrong.
Regards,
Andrii.
On Fri, Aug 31, 2018 at 10:24 AM Ian Romanick wrote:
> On 08/30/2018 07:42 PM, Marek Olšák wrote:
> > Sadly, there are no tests for this.
>
> Ok... I just sent one to the piglit list. It should be easy enough to
> create a few more for other glDeleteFoo functions. I'll save that for
> during boring meetings. ;)
>
> > Marek
> >
> > On Thu, Aug 30, 2018 at 6:24 PM, Ian Romanick
> wrote:
> >> This patch is
> >>
> >> Reviewed-by: Ian Romanick
> >>
> >> Is there a piglit test? I wonder how many other glDeleteFoo functions
> >> mishandle the id=0 case. :(
> >>
> >> On 08/30/2018 12:16 PM, Marek Olšák wrote:
> >>> From: Marek Olšák
> >>>
> >>> This fixes a firefox crash.
> >>>
> >>> Fixes: 781a78914c798dc64005b37c6ca1224ce06803fc
> >>> ---
> >>> src/mesa/main/arrayobj.c | 4
> >>> 1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
> >>> index a23031fe182..6e0665c0e5d 100644
> >>> --- a/src/mesa/main/arrayobj.c
> >>> +++ b/src/mesa/main/arrayobj.c
> >>> @@ -1007,20 +1007,24 @@ _mesa_BindVertexArray(GLuint id)
> >>> *
> >>> * \param n Number of array objects to delete.
> >>> * \param idsArray of \c n array object IDs.
> >>> */
> >>> static void
> >>> delete_vertex_arrays(struct gl_context *ctx, GLsizei n, const GLuint
> *ids)
> >>> {
> >>> GLsizei i;
> >>>
> >>> for (i = 0; i < n; i++) {
> >>> + /* IDs equal to 0 should be silently ignored. */
> >>> + if (!ids[i])
> >>> + continue;
> >>> +
> >>>struct gl_vertex_array_object *obj = _mesa_lookup_vao(ctx,
> ids[i]);
> >>>
> >>>if (obj) {
> >>> assert(obj->Name == ids[i]);
> >>>
> >>> /* If the array object is currently bound, the spec says
> "the binding
> >>>* for that object reverts to zero and the default vertex
> array
> >>>* becomes current."
> >>>*/
> >>> if (obj == ctx->Array.VAO)
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: ignore VAO IDs equal to 0 in glDeleteVertexArrays
On 08/30/2018 07:42 PM, Marek Olšák wrote:
> Sadly, there are no tests for this.
Ok... I just sent one to the piglit list. It should be easy enough to
create a few more for other glDeleteFoo functions. I'll save that for
during boring meetings. ;)
> Marek
>
> On Thu, Aug 30, 2018 at 6:24 PM, Ian Romanick wrote:
>> This patch is
>>
>> Reviewed-by: Ian Romanick
>>
>> Is there a piglit test? I wonder how many other glDeleteFoo functions
>> mishandle the id=0 case. :(
>>
>> On 08/30/2018 12:16 PM, Marek Olšák wrote:
>>> From: Marek Olšák
>>>
>>> This fixes a firefox crash.
>>>
>>> Fixes: 781a78914c798dc64005b37c6ca1224ce06803fc
>>> ---
>>> src/mesa/main/arrayobj.c | 4
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
>>> index a23031fe182..6e0665c0e5d 100644
>>> --- a/src/mesa/main/arrayobj.c
>>> +++ b/src/mesa/main/arrayobj.c
>>> @@ -1007,20 +1007,24 @@ _mesa_BindVertexArray(GLuint id)
>>> *
>>> * \param n Number of array objects to delete.
>>> * \param idsArray of \c n array object IDs.
>>> */
>>> static void
>>> delete_vertex_arrays(struct gl_context *ctx, GLsizei n, const GLuint *ids)
>>> {
>>> GLsizei i;
>>>
>>> for (i = 0; i < n; i++) {
>>> + /* IDs equal to 0 should be silently ignored. */
>>> + if (!ids[i])
>>> + continue;
>>> +
>>>struct gl_vertex_array_object *obj = _mesa_lookup_vao(ctx, ids[i]);
>>>
>>>if (obj) {
>>> assert(obj->Name == ids[i]);
>>>
>>> /* If the array object is currently bound, the spec says "the
>>> binding
>>>* for that object reverts to zero and the default vertex array
>>>* becomes current."
>>>*/
>>> if (obj == ctx->Array.VAO)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: ignore VAO IDs equal to 0 in glDeleteVertexArrays
Sadly, there are no tests for this.
Marek
On Thu, Aug 30, 2018 at 6:24 PM, Ian Romanick wrote:
> This patch is
>
> Reviewed-by: Ian Romanick
>
> Is there a piglit test? I wonder how many other glDeleteFoo functions
> mishandle the id=0 case. :(
>
> On 08/30/2018 12:16 PM, Marek Olšák wrote:
>> From: Marek Olšák
>>
>> This fixes a firefox crash.
>>
>> Fixes: 781a78914c798dc64005b37c6ca1224ce06803fc
>> ---
>> src/mesa/main/arrayobj.c | 4
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
>> index a23031fe182..6e0665c0e5d 100644
>> --- a/src/mesa/main/arrayobj.c
>> +++ b/src/mesa/main/arrayobj.c
>> @@ -1007,20 +1007,24 @@ _mesa_BindVertexArray(GLuint id)
>> *
>> * \param n Number of array objects to delete.
>> * \param idsArray of \c n array object IDs.
>> */
>> static void
>> delete_vertex_arrays(struct gl_context *ctx, GLsizei n, const GLuint *ids)
>> {
>> GLsizei i;
>>
>> for (i = 0; i < n; i++) {
>> + /* IDs equal to 0 should be silently ignored. */
>> + if (!ids[i])
>> + continue;
>> +
>>struct gl_vertex_array_object *obj = _mesa_lookup_vao(ctx, ids[i]);
>>
>>if (obj) {
>> assert(obj->Name == ids[i]);
>>
>> /* If the array object is currently bound, the spec says "the
>> binding
>>* for that object reverts to zero and the default vertex array
>>* becomes current."
>>*/
>> if (obj == ctx->Array.VAO)
>>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: ignore VAO IDs equal to 0 in glDeleteVertexArrays
This patch is
Reviewed-by: Ian Romanick
Is there a piglit test? I wonder how many other glDeleteFoo functions
mishandle the id=0 case. :(
On 08/30/2018 12:16 PM, Marek Olšák wrote:
> From: Marek Olšák
>
> This fixes a firefox crash.
>
> Fixes: 781a78914c798dc64005b37c6ca1224ce06803fc
> ---
> src/mesa/main/arrayobj.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
> index a23031fe182..6e0665c0e5d 100644
> --- a/src/mesa/main/arrayobj.c
> +++ b/src/mesa/main/arrayobj.c
> @@ -1007,20 +1007,24 @@ _mesa_BindVertexArray(GLuint id)
> *
> * \param n Number of array objects to delete.
> * \param idsArray of \c n array object IDs.
> */
> static void
> delete_vertex_arrays(struct gl_context *ctx, GLsizei n, const GLuint *ids)
> {
> GLsizei i;
>
> for (i = 0; i < n; i++) {
> + /* IDs equal to 0 should be silently ignored. */
> + if (!ids[i])
> + continue;
> +
>struct gl_vertex_array_object *obj = _mesa_lookup_vao(ctx, ids[i]);
>
>if (obj) {
> assert(obj->Name == ids[i]);
>
> /* If the array object is currently bound, the spec says "the
> binding
>* for that object reverts to zero and the default vertex array
>* becomes current."
>*/
> if (obj == ctx->Array.VAO)
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa: ignore VAO IDs equal to 0 in glDeleteVertexArrays
From: Marek Olšák
This fixes a firefox crash.
Fixes: 781a78914c798dc64005b37c6ca1224ce06803fc
---
src/mesa/main/arrayobj.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
index a23031fe182..6e0665c0e5d 100644
--- a/src/mesa/main/arrayobj.c
+++ b/src/mesa/main/arrayobj.c
@@ -1007,20 +1007,24 @@ _mesa_BindVertexArray(GLuint id)
*
* \param n Number of array objects to delete.
* \param idsArray of \c n array object IDs.
*/
static void
delete_vertex_arrays(struct gl_context *ctx, GLsizei n, const GLuint *ids)
{
GLsizei i;
for (i = 0; i < n; i++) {
+ /* IDs equal to 0 should be silently ignored. */
+ if (!ids[i])
+ continue;
+
struct gl_vertex_array_object *obj = _mesa_lookup_vao(ctx, ids[i]);
if (obj) {
assert(obj->Name == ids[i]);
/* If the array object is currently bound, the spec says "the binding
* for that object reverts to zero and the default vertex array
* becomes current."
*/
if (obj == ctx->Array.VAO)
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
