v2: In v1 &q->Ready was passed into query_store_result which could cause segfault in memcpy. (q->Ready is bool of size 1, while size parameter could be 4 or 8) Now store result in local variable and pass pointer to it instead.
Signed-off-by: Rafal Mielniczuk <rafal.mielnicz...@gmail.com> --- src/mesa/main/queryobj.c | 89 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 75 insertions(+), 14 deletions(-) diff --git a/src/mesa/main/queryobj.c b/src/mesa/main/queryobj.c index ba9c7f5..12a7af1 100644 --- a/src/mesa/main/queryobj.c +++ b/src/mesa/main/queryobj.c @@ -188,6 +188,51 @@ get_query_binding_point(struct gl_context *ctx, GLenum target) } +/** + * This function is called by GetQueryObject{ui64}v functions. + * If QueryBuffer is bound, it stores the result in it, + * otherwise it copies the value into params location. + * + * Storing a value in QueryBuffer using this function is suboptimal + * as it introduces GPU-CPU-GPU roundtrip. Drivers should implement + * this functionality internally, if possible. + */ +static void +query_store_result(struct gl_context *ctx, void *params, + GLuint size, void *data, const char *caller) +{ + /* Page 44 of the OpenGL 4.4 spec says: + * + * "Initially, zero is bound to the QUERY_BUFFER binding point, + * indicating that params is a pointer into client memory. + * However, if a non-zero buffer object is bound as the current query + * result buffer (see section 6.1), then params is treated as an offset + * into the designated buffer object." + */ + if (ctx->Extensions.ARB_query_buffer_object + && ctx->QueryBuffer != ctx->Shared->NullBufferObj) { + GLsizeiptr offset = (GLsizeiptr)params; + + /* ARB_query_buffer_object spec says: + * + * "An INVALID_OPERATION error is generated if the command would + * cause data to be written beyond the bounds of the buffer currently + * bound to the QUERY_BUFFER target." + */ + if (offset + size > ctx->QueryBuffer->Size) { + _mesa_error(ctx, GL_INVALID_OPERATION, + "%s(buffer bounds exceeded)", caller); + return; + } + + ctx->Driver.BufferSubData(ctx, offset, size, data, + ctx->QueryBuffer); + } else { + memcpy(params, data, size); + } +} + + void GLAPIENTRY _mesa_GenQueries(GLsizei n, GLuint *ids) { @@ -578,6 +623,7 @@ void GLAPIENTRY _mesa_GetQueryObjectiv(GLuint id, GLenum pname, GLint *params) { struct gl_query_object *q = NULL; + GLint result = 0; GET_CURRENT_CONTEXT(ctx); if (MESA_VERBOSE & VERBOSE_API) @@ -605,27 +651,30 @@ _mesa_GetQueryObjectiv(GLuint id, GLenum pname, GLint *params) if (q->Target == GL_ANY_SAMPLES_PASSED || q->Target == GL_ANY_SAMPLES_PASSED_CONSERVATIVE) { if (q->Result) - *params = GL_TRUE; + result = GL_TRUE; else - *params = GL_FALSE; + result = GL_FALSE; } else { if (q->Result > 0x7fffffff) { - *params = 0x7fffffff; + result = 0x7fffffff; } else { - *params = (GLint)q->Result; + result = (GLint)q->Result; } } break; case GL_QUERY_RESULT_AVAILABLE_ARB: if (!q->Ready) ctx->Driver.CheckQuery( ctx, q ); - *params = q->Ready; + result = q->Ready; break; default: _mesa_error(ctx, GL_INVALID_ENUM, "glGetQueryObjectivARB(pname)"); return; } + + query_store_result(ctx, params, sizeof(result), &result, + "glGetQueryObjectivARB"); } @@ -633,6 +682,7 @@ void GLAPIENTRY _mesa_GetQueryObjectuiv(GLuint id, GLenum pname, GLuint *params) { struct gl_query_object *q = NULL; + GLuint result = 0; GET_CURRENT_CONTEXT(ctx); if (MESA_VERBOSE & VERBOSE_API) @@ -660,27 +710,30 @@ _mesa_GetQueryObjectuiv(GLuint id, GLenum pname, GLuint *params) if (q->Target == GL_ANY_SAMPLES_PASSED || q->Target == GL_ANY_SAMPLES_PASSED_CONSERVATIVE) { if (q->Result) - *params = GL_TRUE; + result = GL_TRUE; else - *params = GL_FALSE; + result = GL_FALSE; } else { if (q->Result > 0xffffffff) { - *params = 0xffffffff; + result = 0xffffffff; } else { - *params = (GLuint)q->Result; + result = (GLuint)q->Result; } } break; case GL_QUERY_RESULT_AVAILABLE_ARB: if (!q->Ready) ctx->Driver.CheckQuery( ctx, q ); - *params = q->Ready; + result = q->Ready; break; default: _mesa_error(ctx, GL_INVALID_ENUM, "glGetQueryObjectuivARB(pname)"); return; } + + query_store_result(ctx, params, sizeof(result), &result, + "glGetQueryObjectuivARB"); } @@ -691,6 +744,7 @@ void GLAPIENTRY _mesa_GetQueryObjecti64v(GLuint id, GLenum pname, GLint64EXT *params) { struct gl_query_object *q = NULL; + GLint64EXT result = 0; GET_CURRENT_CONTEXT(ctx); if (MESA_VERBOSE & VERBOSE_API) @@ -714,17 +768,20 @@ _mesa_GetQueryObjecti64v(GLuint id, GLenum pname, GLint64EXT *params) case GL_QUERY_RESULT_ARB: if (!q->Ready) ctx->Driver.WaitQuery(ctx, q); - *params = q->Result; + result = q->Result; break; case GL_QUERY_RESULT_AVAILABLE_ARB: if (!q->Ready) ctx->Driver.CheckQuery( ctx, q ); - *params = q->Ready; + result = q->Ready; break; default: _mesa_error(ctx, GL_INVALID_ENUM, "glGetQueryObjecti64vARB(pname)"); return; } + + query_store_result(ctx, params, sizeof(result), &result, + "glGetQueryObjecti64vARB"); } @@ -735,6 +792,7 @@ void GLAPIENTRY _mesa_GetQueryObjectui64v(GLuint id, GLenum pname, GLuint64EXT *params) { struct gl_query_object *q = NULL; + GLuint64EXT result = 0; GET_CURRENT_CONTEXT(ctx); if (MESA_VERBOSE & VERBOSE_API) @@ -758,17 +816,20 @@ _mesa_GetQueryObjectui64v(GLuint id, GLenum pname, GLuint64EXT *params) case GL_QUERY_RESULT_ARB: if (!q->Ready) ctx->Driver.WaitQuery(ctx, q); - *params = q->Result; + result = q->Result; break; case GL_QUERY_RESULT_AVAILABLE_ARB: if (!q->Ready) ctx->Driver.CheckQuery( ctx, q ); - *params = q->Ready; + result = q->Ready; break; default: _mesa_error(ctx, GL_INVALID_ENUM, "glGetQueryObjectui64vARB(pname)"); return; } + + query_store_result(ctx, params, sizeof(result), &result, + "glGetQueryObjectui64vARB"); } /** -- 1.9.0 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev