[Mesa-dev] [PATCH v2 5/6] mesa: Fix corner cases of BindBufferBase with transform feedback.
Paul Berry
stereotype441 at gmail.com
Sat Dec 15 22:09:24 PST 2012
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.
Cc: Eric Anholt <eric at anholt.net>
---
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 f3dbda2..273a79f 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 18ab2db..96b84d8 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 4782a6f..dd03038 100644
--- a/src/mesa/main/transformfeedback.c
+++ b/src/mesa/main/transformfeedback.c
@@ -247,6 +247,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.
*/
@@ -337,6 +386,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
@@ -407,7 +458,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;
}
@@ -466,7 +517,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;
@@ -481,12 +531,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);
}
@@ -502,7 +547,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)");
@@ -542,12 +586,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);
}
--
1.8.0.2
More information about the mesa-dev
mailing list