On Tue, 2016-04-05 at 14:27 +0300, kevin.rogo...@intel.com wrote:
> From: Kevin Rogovin <kevin.rogo...@intel.com>
>
> The extension, GL_ARB_occlusion_queries mandates that an
> INVALID_OPERATION should be thrown if DeleteQueriesARB is
> called while a query is active. In contrast, the GL spec
> has no such requirement for DeleteQueries. This patch
> unaliases the two functions and has that the ARB variant
> performs that extra check.
>
> Signed-off-by: Kevin Rogovin <kevin.rogo...@intel.com>
> ---
> src/mapi/glapi/gen/gl_API.xml | 2 +-
> src/mesa/main/context.c | 1 +
> src/mesa/main/queryobj.c | 31 ++++++++++++++++++++++++++-----
> src/mesa/main/queryobj.h | 2 ++
> 4 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
> index 5918e63..452584c 100644
> --- a/src/mapi/glapi/gen/gl_API.xml
> +++ b/src/mapi/glapi/gen/gl_API.xml
> @@ -7569,7 +7569,7 @@
> <param name="ids" type="GLuint *"/>
> </function>
>
> - <function name="DeleteQueriesARB" alias="DeleteQueries">
> + <function name="DeleteQueriesARB">
> <param name="n" type="GLsizei"/>
> <param name="ids" type="const GLuint *"/>
> </function>
> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
> index dbba136..1b250c0 100644
> --- a/src/mesa/main/context.c
> +++ b/src/mesa/main/context.c
> @@ -1090,6 +1090,7 @@ create_beginend_table(const struct gl_context *ctx)
> COPY_DISPATCH(IsTexture);
> COPY_DISPATCH(IsTransformFeedback);
> COPY_DISPATCH(DeleteQueries);
> + COPY_DISPATCH(DeleteQueriesARB);
> COPY_DISPATCH(AreTexturesResident);
> COPY_DISPATCH(FenceSync);
> COPY_DISPATCH(ClientWaitSync);
> diff --git a/src/mesa/main/queryobj.c b/src/mesa/main/queryobj.c
> index 43f1b0c..d8575c2 100644
> --- a/src/mesa/main/queryobj.c
> +++ b/src/mesa/main/queryobj.c
> @@ -321,19 +321,29 @@ _mesa_CreateQueries(GLenum target, GLsizei n, GLuint
> *ids)
> create_queries(ctx, target, n, ids, true);
> }
>
> -
> -void GLAPIENTRY
> -_mesa_DeleteQueries(GLsizei n, const GLuint *ids)
> +static void
> +delete_queries(GLsizei n, const GLuint *ids, const char *func, bool is_arb)
> {
> GLint i;
> GET_CURRENT_CONTEXT(ctx);
> FLUSH_VERTICES(ctx, 0);
>
> if (MESA_VERBOSE & VERBOSE_API)
> - _mesa_debug(ctx, "glDeleteQueries(%d)\n", n);
> + _mesa_debug(ctx, "%s(%d)\n", func, n);
>
> if (n < 0) {
> - _mesa_error(ctx, GL_INVALID_VALUE, "glDeleteQueriesARB(n < 0)");
> + _mesa_error(ctx, GL_INVALID_VALUE, "%s(n < 0)", func);
> + return;
> + }
> +
> + /* From GL_ARB_occlusion_query:
> + "The error INVALID_OPERATION is generated if GenQueriesARB or
> + DeleteQueriesARB is called when a query of any target is active."
> + That extension only support the target SAMPLES_PASSED_ARB, so we
... only supports ...
Same typo in the other patch.
> + only check for CurrentOcclusionObject
> + */
> + if(is_arb && ctx->Query.CurrentOcclusionObject != NULL) {
> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s\n", func);
> return;
> }
>
> @@ -358,6 +368,17 @@ _mesa_DeleteQueries(GLsizei n, const GLuint *ids)
> }
> }
>
> +void GLAPIENTRY
> +_mesa_DeleteQueries(GLsizei n, const GLuint *ids)
> +{
> + delete_queries(n, ids, "glDeleteQueries", false);
> +}
> +
> +void GLAPIENTRY
> +_mesa_DeleteQueriesARB(GLsizei n, const GLuint *ids)
> +{
> + delete_queries(n, ids, "glDeleteQueriesARB", true);
> +}
Maybe you could only add the boolean parameter and decide the function
name inside delete_queries() based on that, it is a bit redundant
otherwise.
Also, you solve this slightly different for GenQueries, where we add the
ARB-specific check inside the ARB function, why not just use the same
approach for both to keep consistency? create_queries() already decides
the funcion name inside based on a bool parameter too.
Either way, the series is:
Reviewed-by: Iago Toral Quiroga <ito...@igalia.com>
> GLboolean GLAPIENTRY
> _mesa_IsQuery(GLuint id)
> diff --git a/src/mesa/main/queryobj.h b/src/mesa/main/queryobj.h
> index 245d104..524767e 100644
> --- a/src/mesa/main/queryobj.h
> +++ b/src/mesa/main/queryobj.h
> @@ -56,6 +56,8 @@ void GLAPIENTRY
> _mesa_CreateQueries(GLenum target, GLsizei n, GLuint *ids);
> void GLAPIENTRY
> _mesa_DeleteQueries(GLsizei n, const GLuint *ids);
> +void GLAPIENTRY
> +_mesa_DeleteQueriesARB(GLsizei n, const GLuint *ids);
> GLboolean GLAPIENTRY
> _mesa_IsQuery(GLuint id);
> void GLAPIENTRY
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
