From 1ad516207d2297b14fe26627e5bb599a9f704ac5 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Sat, 15 Dec 2012 13:06:10 -0800 Subject: [PATCH] mesa: Fix corner cases of BindBufferBase with transform feedback. This patch implements the following behaviours, which are mandated by the GL 4.3 and GLES3 specs. 1. Regarding the GL_TRANSFORM_FEEDBACK_BUFFER_SIZE query: "If the ... size was not specified when the buffer object was bound (e.g. if it was bound with BindBufferBase), ... zero is returned." (GL 4.3 section 6.7.1 "Indexed Buffer Object Limits and Binding Queries"). 2. "BindBufferBase binds the entire buffer, even when the size of the buffer is changed after the binding is established. It is equivalent to calling BindBufferRange with offset zero, while size is determined by the size of the bound buffer at the time the binding is used." (GL 4.3 section 6.1.1 "Binding Buffer Objects to Indexed Targets"). I interpret "at the time the binding is used" to mean "at the time of the call to glBeginTransformFeedback". 3. "Regardless of the size specified with BindBufferRange, or indirectly with BindBufferBase, the GL will never read or write beyond the end of a bound buffer. In some cases this constraint may result in visibly different behavior when a buffer overflow would otherwise result, such as described for transform feedback operations in section 13.2.2." (GL 4.3 section 6.1.1 "Binding Buffer Objects to Indexed Targets"). Item 1 has been part of the spec all the way back to the inception of the EXT_transform_feedback extension. Items 2 and 3 were added in GL 4.2 and GLES 3. Prior to GL 4.2, in place of items 2 and 3, the spec simply said "BindBufferBase is equivalent to calling BindBufferRange with offset zero and size equal to the size of buffer." For transform feedback, Mesa behaved as though this meant "...equal to the size of buffer at the time of the call to BindBufferBase". However, this was problematic because it left it ambiguous what to do if the buffer is shrunk between the call to BindBuffer{Base,Range} and the call to BeginTransformFeedback. Prior to this patch, Mesa's behaviour was to try to write beyond the end of the buffer, likely resulting in memory corruption. In light of this, I'm interpreting the spec change as a clarification, not an intended behavioural change, so I'm making the change apply regardless of API version. Fixes GLES3 conformance test transform_feedback2_pause_resume.test. Reviewed-by: Jordan Justen --- src/mesa/main/get.c | 3 +- src/mesa/main/mtypes.h | 13 +++++- src/mesa/main/transformfeedback.c | 69 ++++++++++++++++++++++++------- 3 files changed, 68 insertions(+), 17 deletions(-) diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c index f3dbda2d3d2..273a79f7fb7 100644 --- a/src/mesa/main/get.c +++ b/src/mesa/main/get.c @@ -1574,7 +1574,8 @@ find_value_indexed(const char *func, GLenum pname, GLuint index, union value *v) goto invalid_value; if (!ctx->Extensions.EXT_transform_feedback) goto invalid_enum; - v->value_int64 = ctx->TransformFeedback.CurrentObject->Size[index]; + v->value_int64 + = ctx->TransformFeedback.CurrentObject->RequestedSize[index]; return TYPE_INT64; case GL_TRANSFORM_FEEDBACK_BUFFER_BINDING: diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index 0c201d08f46..097cdc14f58 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -1825,8 +1825,19 @@ struct gl_transform_feedback_object /** Start of feedback data in dest buffer */ GLintptr Offset[MAX_FEEDBACK_BUFFERS]; - /** Max data to put into dest buffer (in bytes) */ + + /** + * Max data to put into dest buffer (in bytes). Computed based on + * RequestedSize and the actual size of the buffer. + */ GLsizeiptr Size[MAX_FEEDBACK_BUFFERS]; + + /** + * Size that was specified when the buffer was bound. If the buffer was + * bound with glBindBufferBase() or glBindBufferOffsetEXT(), this value is + * zero. + */ + GLsizeiptr RequestedSize[MAX_FEEDBACK_BUFFERS]; }; diff --git a/src/mesa/main/transformfeedback.c b/src/mesa/main/transformfeedback.c index b2818caa591..61f2f4f11d0 100644 --- a/src/mesa/main/transformfeedback.c +++ b/src/mesa/main/transformfeedback.c @@ -246,6 +246,55 @@ _mesa_init_transform_feedback_functions(struct dd_function_table *driver) } +/** + * Fill in the correct Size value for each buffer in \c obj. + * + * From the GL 4.3 spec, section 6.1.1 ("Binding Buffer Objects to Indexed + * Targets"): + * + * BindBufferBase binds the entire buffer, even when the size of the buffer + * is changed after the binding is established. It is equivalent to calling + * BindBufferRange with offset zero, while size is determined by the size of + * the bound buffer at the time the binding is used. + * + * Regardless of the size specified with BindBufferRange, or indirectly with + * BindBufferBase, the GL will never read or write beyond the end of a bound + * buffer. In some cases this constraint may result in visibly different + * behavior when a buffer overflow would otherwise result, such as described + * for transform feedback operations in section 13.2.2. + */ +static void +compute_transform_feedback_buffer_sizes( + struct gl_transform_feedback_object *obj) +{ + unsigned i = 0; + for (i = 0; i < MAX_FEEDBACK_BUFFERS; ++i) { + GLintptr offset = obj->Offset[i]; + GLsizeiptr buffer_size + = obj->Buffers[i] == NULL ? 0 : obj->Buffers[i]->Size; + GLsizeiptr available_space + = buffer_size <= offset ? 0 : buffer_size - offset; + GLsizeiptr computed_size; + if (obj->RequestedSize[i] == 0) { + /* No size was specified at the time the buffer was bound, so allow + * writing to all available space in the buffer. + */ + computed_size = available_space; + } else { + /* A size was specified at the time the buffer was bound, however + * it's possible that the buffer has shrunk since then. So only + * allow writing to the minimum of the specified size and the space + * available. + */ + computed_size = MIN2(available_space, obj->RequestedSize[i]); + } + + /* Legal sizes must be multiples of four, so round down if necessary. */ + obj->Size[i] = computed_size & ~0x3; + } +} + + /** * Compute the maximum number of vertices that can be written to the currently * enabled transform feedback buffers without overflowing any of them. @@ -338,6 +387,8 @@ _mesa_BeginTransformFeedback(GLenum mode) obj->Active = GL_TRUE; ctx->TransformFeedback.Mode = mode; + compute_transform_feedback_buffer_sizes(obj); + if (_mesa_is_gles3(ctx)) { /* In GLES3, we are required to track the usage of the transform * feedback buffer and report INVALID_OPERATION if a draw call tries to @@ -408,7 +459,7 @@ bind_buffer_range(struct gl_context *ctx, GLuint index, obj->BufferNames[index] = bufObj->Name; obj->Offset[index] = offset; - obj->Size[index] = size; + obj->RequestedSize[index] = size; } @@ -467,7 +518,6 @@ _mesa_bind_buffer_base_transform_feedback(struct gl_context *ctx, struct gl_buffer_object *bufObj) { struct gl_transform_feedback_object *obj; - GLsizeiptr size; obj = ctx->TransformFeedback.CurrentObject; @@ -482,12 +532,7 @@ _mesa_bind_buffer_base_transform_feedback(struct gl_context *ctx, return; } - /* default size is the buffer size rounded down to nearest - * multiple of four. - */ - size = bufObj->Size & ~0x3; - - bind_buffer_range(ctx, index, bufObj, 0, size); + bind_buffer_range(ctx, index, bufObj, 0, 0); } @@ -503,7 +548,6 @@ _mesa_BindBufferOffsetEXT(GLenum target, GLuint index, GLuint buffer, struct gl_transform_feedback_object *obj; struct gl_buffer_object *bufObj; GET_CURRENT_CONTEXT(ctx); - GLsizeiptr size; if (target != GL_TRANSFORM_FEEDBACK_BUFFER) { _mesa_error(ctx, GL_INVALID_ENUM, "glBindBufferOffsetEXT(target)"); @@ -543,12 +587,7 @@ _mesa_BindBufferOffsetEXT(GLenum target, GLuint index, GLuint buffer, return; } - /* default size is the buffer size rounded down to nearest - * multiple of four. - */ - size = (bufObj->Size - offset) & ~0x3; - - bind_buffer_range(ctx, index, bufObj, offset, size); + bind_buffer_range(ctx, index, bufObj, offset, 0); } -- 2.30.2