On 27/02/15 01:20, Laura Ekstrand wrote:


On Mon, Feb 16, 2015 at 6:14 AM, Martin Peres <martin.pe...@linux.intel.com <mailto:martin.pe...@linux.intel.com>> wrote:

    Signed-off-by: Martin Peres <martin.pe...@linux.intel.com
    <mailto:martin.pe...@linux.intel.com>>
    ---
     src/mapi/glapi/gen/ARB_direct_state_access.xml |  8 +++
     src/mesa/main/queryobj.c                       | 76
    +++++++++++++++++++++++---
     src/mesa/main/queryobj.h                       |  2 +
     src/mesa/main/tests/dispatch_sanity.cpp        |  1 +
     4 files changed, 80 insertions(+), 7 deletions(-)

    diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml
    b/src/mapi/glapi/gen/ARB_direct_state_access.xml
    index 340dbba..652e8bc 100644
    --- a/src/mapi/glapi/gen/ARB_direct_state_access.xml
    +++ b/src/mapi/glapi/gen/ARB_direct_state_access.xml
    @@ -308,5 +308,13 @@
           <param name="params" type="GLint *" />
        </function>

    +   <!-- Query object functions -->
    +
    +   <function name="CreateQueries" offset="assign">
    +      <param name="target" type="GLenum" />
    +      <param name="n" type="GLsizei" />
    +      <param name="ids" type="GLuint *" />
    +   </function>
    +
     </category>
     </OpenGLAPI>
    diff --git a/src/mesa/main/queryobj.c b/src/mesa/main/queryobj.c
    index 17eaaac..1bb74c9 100644
    --- a/src/mesa/main/queryobj.c
    +++ b/src/mesa/main/queryobj.c
    @@ -188,18 +188,22 @@ get_query_binding_point(struct gl_context
    *ctx, GLenum target, GLuint index)
        }
     }

    -
    -void GLAPIENTRY
    -_mesa_GenQueries(GLsizei n, GLuint *ids)
    +/**
    + * Create $n query objects and store them in *ids. Make them of
    type $target
    + * if dsa is set. Called from _mesa_GenQueries() and
    _mesa_CreateQueries().
    + */
    +static void
    +create_queries(struct gl_context *ctx, GLenum target, GLsizei n,
    GLuint *ids,
    +               bool dsa)
     {
    +   const char *func = dsa ? "glGenQueries" : "glCreateQueries";
        GLuint first;
    -   GET_CURRENT_CONTEXT(ctx);

        if (MESA_VERBOSE & VERBOSE_API)
    -      _mesa_debug(ctx, "glGenQueries(%d)\n", n);
    +      _mesa_debug(ctx, "%s(%d)\n", func, n);

        if (n < 0) {
    -      _mesa_error(ctx, GL_INVALID_VALUE, "glGenQueriesARB(n < 0)");
    +      _mesa_error(ctx, GL_INVALID_VALUE, "%s(n < 0)", func);
           return;
        }

    @@ -210,8 +214,12 @@ _mesa_GenQueries(GLsizei n, GLuint *ids)
              struct gl_query_object *q
                 = ctx->Driver.NewQueryObject(ctx, first + i);
              if (!q) {
    -            _mesa_error(ctx, GL_OUT_OF_MEMORY, "glGenQueriesARB");
    +            _mesa_error(ctx, GL_OUT_OF_MEMORY, "%s", func);
                 return;
    +         } else if (dsa) {
    +            /* Do the equivalent of binding the buffer with a
    target */
    +            q->Target = target;
    +            q->EverBound = GL_TRUE;
              }
              ids[i] = first + i;
              _mesa_HashInsert(ctx->Query.QueryObjects, first + i, q);
    @@ -219,6 +227,36 @@ _mesa_GenQueries(GLsizei n, GLuint *ids)
        }
     }

    +void GLAPIENTRY
    +_mesa_GenQueries(GLsizei n, GLuint *ids)
    +{
    +   GET_CURRENT_CONTEXT(ctx);
    +   create_queries(ctx, 0, n, ids, false);
    +}
    +
    +void GLAPIENTRY
    +_mesa_CreateQueries(GLenum target, GLsizei n, GLuint *ids)
    +{
    +   GET_CURRENT_CONTEXT(ctx);
    +
    +   switch (target) {
    +   case GL_SAMPLES_PASSED:
    +   case GL_ANY_SAMPLES_PASSED:
    +   case GL_ANY_SAMPLES_PASSED_CONSERVATIVE:
    +   case GL_TIME_ELAPSED:
    +   case GL_TIMESTAMP:
    +   case GL_PRIMITIVES_GENERATED:
    +   case GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN:
    +      break;
    +   default:
    +      _mesa_error(ctx, GL_INVALID_ENUM, "glCreateQueries(invalid
    target = %i)",
    +                  target);

I think it would be nicer to have "invalid target = %s", _mesa_lookup_enum_by_nr(target) in your error message, because otherwise the user might have to go looking through Mesa to find out which incorrect target they passed. For example, not everyone knows that 3553 = GL_TEXTURE_2D. (Although I do, for some sick reason :))

Indeed, this is much nicer like that! However, why do you know that GL_TEXTURE_2D == 3553? Isn't it the point of enums not to have to care about this? :D


    +      return;
    +   }
    +
    +   create_queries(ctx, target, n, ids, true);
    +}
    +

     void GLAPIENTRY
     _mesa_DeleteQueries(GLsizei n, const GLuint *ids)
    @@ -363,6 +401,18 @@ _mesa_BeginQueryIndexed(GLenum target, GLuint
    index, GLuint id)
           }
        }

    +   /* This possibly changes the target of a buffer allocated by
    +    * CreateQueries. Issue 39) in the ARB_direct_state_access
    extension states
    +    * the following:
    +    *
    +    * "CreateQueries adds a <target>, so strictly speaking the
    <target>
    +    * command isn't needed for BeginQuery/EndQuery, but in the
    end, this also
    +    * isn't a selector, so we decided not to change it."
    +    *
    +    * Updating the target of the query object should be
    acceptable, so let's
    +    * do that.
    +    */
    +
        q->Target = target;
        q->Active = GL_TRUE;
        q->Result = 0;
    @@ -480,6 +530,18 @@ _mesa_QueryCounter(GLuint id, GLenum target)
           return;
        }

    +   /* This possibly changes the target of a buffer allocated by
    +    * CreateQueries. Issue 39) in the ARB_direct_state_access
    extension states
    +    * the following:
    +    *
    +    * "CreateQueries adds a <target>, so strictly speaking the
    <target>
    +    * command isn't needed for BeginQuery/EndQuery, but in the
    end, this also
    +    * isn't a selector, so we decided not to change it."
    +    *
    +    * Updating the target of the query object should be
    acceptable, so let's
    +    * do that.
    +    */
    +
        q->Target = target;
        q->Result = 0;
        q->Ready = GL_FALSE;
    diff --git a/src/mesa/main/queryobj.h b/src/mesa/main/queryobj.h
    index 6cbcabd..431d420 100644
    --- a/src/mesa/main/queryobj.h
    +++ b/src/mesa/main/queryobj.h
    @@ -51,6 +51,8 @@ _mesa_free_queryobj_data(struct gl_context *ctx);
     void GLAPIENTRY
     _mesa_GenQueries(GLsizei n, GLuint *ids);
     void GLAPIENTRY
    +_mesa_CreateQueries(GLenum target, GLsizei n, GLuint *ids);
    +void GLAPIENTRY
     _mesa_DeleteQueries(GLsizei n, const GLuint *ids);
     GLboolean GLAPIENTRY
     _mesa_IsQuery(GLuint id);
    diff --git a/src/mesa/main/tests/dispatch_sanity.cpp
    b/src/mesa/main/tests/dispatch_sanity.cpp
    index ad5da83..ee448f1 100644
    --- a/src/mesa/main/tests/dispatch_sanity.cpp
    +++ b/src/mesa/main/tests/dispatch_sanity.cpp
    @@ -993,6 +993,7 @@ const struct function
    gl_core_functions_possible[] = {
        { "glTextureStorage2DMultisample", 45, -1 },
        { "glTextureStorage3DMultisample", 45, -1 },
        { "glTextureBuffer", 45, -1 },
    +   { "glCreateQueries", 45, -1 },

        /* GL_EXT_polygon_offset_clamp */
        { "glPolygonOffsetClampEXT", 11, -1 },
    --
    2.3.0

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


Otherwise, looks good to me.

Reviewed-by: Laura Ekstrand <la...@jlekstrand.net <mailto:la...@jlekstrand.net>>


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

Reply via email to