On 10/17/2013 02:35 PM, Carl Worth wrote:
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-sta...@lists.freedesktop.org>
---

Brian Paul <bri...@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);

Valgrind found an invalid pointer (and crashed!) when I modified your piglit test (see other msg). We also need to make sure that the ctx->Query.CurrentFoo binding point is cleared. Something like this:

            if (q->Active) {
               struct gl_query_object **bindpt =
                  get_query_binding_point(ctx, q->Target);
               assert(bindpt);  /* _should_ be non-null if q is active */
               if (bindpt) {
                  *bindpt = NULL;
               }
               ...


+            }
              _mesa_HashRemove(ctx->Query.QueryObjects, ids[i]);
              ctx->Driver.DeleteQuery(ctx, q);
           }



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

Reply via email to