[Mesa-stable] [Mesa-dev] [PATCH] Remove error when calling glGenQueries/glDeleteQueries while a query is active

[Carl Worth]

There is nothing in the OpenGL specification which prevents the user from
calling glGenQueries to generate a new query object while another object is
active. Neither is there anything in the Mesa implementation which prevents
this. So remove the INVALID_OPERATION errors in this case.

Similarly, it is explicitly allowed by the OpenGL specification to delete an
active query, so remove the assertion for that case and be sure to call the
driver's EndQuery hook.

CC: <mesa-stable at lists.freedesktop.org>
---

Brian Paul <brianp at vmware.com> writes:
> On 10/17/2013 12:14 PM, Carl Worth wrote:
> But from http://www.opengl.org/registry/specs/ARB/occlusion_query.txt:
>
> """
>      Calling either GenQueriesARB or DeleteQueriesARB while any query of
>      any target is active causes an INVALID_OPERATION error to be
>      generated.
> """
> (it's about half-way down in the file)  It's also mentioned in the 
> "Errors" section.

Thanks, Brian. That certainly does justify where the original code came
from.

> Maybe that was rescinded since that spec was done.  If so, I'm fine with 
> removing the code.

I can't find any similar error requirement in the OpenGL 4.4 (Core)
specification. (And with the increased number of different query types,
it doesn't seem that the error requirement makes sense.) I did find the
following sentence in the specification (section 4.2):

	If an active query object is deleted its name immediately
	becomes unused, but the underlying object is not deleted until
	it is no longer active.

This sentence presumes the possibility of deleting an active query, so I
think it is reasonable to remove the error.

> However, I wouldn't be surprised if our drivers crashed and burned if an 
> active query is deleted.  gl_query_object isn't referenced counted.

Thanks for the catch. My revised patch below calls the driver's EndQuery
hook, which will hopefully avoid this problem. This does inactivate the
query immediately at delete time, which might seem inconsistent with the
sentence I quoted from the specification above. But I think this is fine
since once the object is deleted the user has no visibility into whether
the object is active or not.

-Carl

 src/mesa/main/queryobj.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/src/mesa/main/queryobj.c b/src/mesa/main/queryobj.c
index a180133..ebdb71c 100644
--- a/src/mesa/main/queryobj.c
+++ b/src/mesa/main/queryobj.c
@@ -202,13 +202,6 @@ _mesa_GenQueries(GLsizei n, GLuint *ids)
       return;
    }
 
-   /* No query objects can be active at this time! */
-   if (ctx->Query.CurrentOcclusionObject ||
-       ctx->Query.CurrentTimerObject) {
-      _mesa_error(ctx, GL_INVALID_OPERATION, "glGenQueriesARB");
-      return;
-   }
-
    first = _mesa_HashFindFreeKeyBlock(ctx->Query.QueryObjects, n);
    if (first) {
       GLsizei i;
@@ -241,18 +234,14 @@ _mesa_DeleteQueries(GLsizei n, const GLuint *ids)
       return;
    }
 
-   /* No query objects can be active at this time! */
-   if (ctx->Query.CurrentOcclusionObject ||
-       ctx->Query.CurrentTimerObject) {
-      _mesa_error(ctx, GL_INVALID_OPERATION, "glDeleteQueriesARB");
-      return;
-   }
-
    for (i = 0; i < n; i++) {
       if (ids[i] > 0) {
          struct gl_query_object *q = _mesa_lookup_query_object(ctx, ids[i]);
          if (q) {
-            ASSERT(!q->Active); /* should be caught earlier */
+            if (q->Active) {
+               q->Active = GL_FALSE;
+               ctx->Driver.EndQuery(ctx, q);
+            }
             _mesa_HashRemove(ctx->Query.QueryObjects, ids[i]);
             ctx->Driver.DeleteQuery(ctx, q);
          }
-- 
1.8.4.rc3

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-stable/attachments/20131017/f6ae6317/attachment.pgp>