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

[Marek Olšák]

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.

Marek