Re: [Mesa-dev] [PATCH 2/7] mesa: implement TIMESTAMP query and glQueryCounter

[Brian Paul]

On 06/27/2012 09:06 AM, Marek Olšák wrote:

On Wed, Jun 27, 2012 at 4:39 PM, Brian Paul<bri...@vmware.com>  wrote:

On 06/26/2012 07:49 PM, Marek Olšák wrote:

---
  src/mesa/main/queryobj.c |   78
++++++++++++++++++++++++++++++++++++++++++----
  1 file changed, 72 insertions(+), 6 deletions(-)

diff --git a/src/mesa/main/queryobj.c b/src/mesa/main/queryobj.c
index f0a9a79..cb50784 100644
--- a/src/mesa/main/queryobj.c
+++ b/src/mesa/main/queryobj.c
@@ -353,9 +353,66 @@ _mesa_EndQueryARB(GLenum target)


  static void GLAPIENTRY
+_mesa_QueryCounter(GLuint id, GLenum target)
+{
+   struct gl_query_object *q;
+   GET_CURRENT_CONTEXT(ctx);
+   ASSERT_OUTSIDE_BEGIN_END(ctx);
+
+   if (MESA_VERBOSE&    VERBOSE_API)

+      _mesa_debug(ctx, "glQueryCounter(%u, %s)\n", id,
+                  _mesa_lookup_enum_by_nr(target));
+
+   /* error checking */
+   if (target != GL_TIMESTAMP) {
+      _mesa_error(ctx, GL_INVALID_ENUM, "glQueryCounter(target)");
+      return;
+   }
+
+   if (id == 0) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glQueryCounter(id==0)");
+      return;
+   }
+
+   q = _mesa_lookup_query_object(ctx, id);
+   if (!q) {
+      /* XXX the Core profile should throw INVALID_OPERATION here */
+
+      /* create new object */
+      q = ctx->Driver.NewQueryObject(ctx, id);
+      if (!q) {
+         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glQueryCounter");
+         return;
+      }
+      _mesa_HashInsert(ctx->Query.QueryObjects, id, q);
+   }
+   else {
+      if (q->Target&&    q->Target != GL_TIMESTAMP) {

+         _mesa_error(ctx, GL_INVALID_OPERATION,
+                     "glQueryCounter(id has an invalid target)");
+         return;
+      }
+   }
+
+   if (q->Active) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glQueryCounter(id is
active)");
+      return;
+   }
+
+   q->Target = target;
+   q->Result = 0;
+   q->Ready = GL_FALSE;
+
+   /* QueryCounter is implemented using EndQuery without BeginQuery
+    * in drivers. This is actually Direct3D and Gallium convention. */
+   ctx->Driver.EndQuery(ctx, q);
+}
+
+
+static void GLAPIENTRY
  _mesa_GetQueryivARB(GLenum target, GLenum pname, GLint *params)
  {
-   struct gl_query_object *q, **bindpt;
+   struct gl_query_object *q = NULL, **bindpt = NULL;
     GET_CURRENT_CONTEXT(ctx);
     ASSERT_OUTSIDE_BEGIN_END(ctx);

@@ -364,13 +421,21 @@ _mesa_GetQueryivARB(GLenum target, GLenum pname,
GLint *params)
                    _mesa_lookup_enum_by_nr(target),
                    _mesa_lookup_enum_by_nr(pname));

-   bindpt = get_query_binding_point(ctx, target);
-   if (!bindpt) {
-      _mesa_error(ctx, GL_INVALID_ENUM, "glGetQueryARB(target)");
-      return;
+   if (target == GL_TIMESTAMP) {
+      if (!ctx->Extensions.ARB_timer_query) {
+         _mesa_error(ctx, GL_INVALID_ENUM, "glGetQueryARB(target)");
+         return;
+      }
     }
+   else {
+      bindpt = get_query_binding_point(ctx, target);
+      if (!bindpt) {
+         _mesa_error(ctx, GL_INVALID_ENUM, "glGetQueryARB(target)");
+         return;
+      }

-   q = *bindpt;
+      q = *bindpt;
+   }

     switch (pname) {
        case GL_QUERY_COUNTER_BITS_ARB:

It looks like if target=GL_TIMESTAMP and pname=GL_QUERY_COUNTER_BITS_ARB
then we'd segfault here since q=NULL:

      case GL_QUERY_COUNTER_BITS_ARB:
         *params = 8 * sizeof(q->Result);
         break;

I thought so too, but the truth is we wouldn't. sizeof(q->Result) is
evaluated at compile time to sizeof(GLuint64). q can be anything and
it won't change the outcome of the expression.

Ah, of course.  Thanks.

-Brian

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