[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<brianp at 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