On 15 December 2012 14:56, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="im">On 12/15/2012 01:27 PM, Paul Berry wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"> On 14 December 2012 13:01, Paul Berry <<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a> </div><div><div class="h5"> <mailto:<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>>> wrote: From the GL 4.3 spec, section 6.7.1 (Indexed Buffer Object Limits and Binding Queries): To query the starting offset or size of the range of a buffer object binding in an indexed array, call GetInteger64i v with target set to respectively the starting offset or binding size name from table 6.5 for that array. index must be in the range zero to the number of bind points supported minus one. If the starting offset or size was not specified when the buffer object was bound (e.g. if it was bound with BindBufferBase), or if no buffer object is bound to the target array at index, zero is returned. Similar language exists in the EXT_transform_feedback spec and in the GLES3 spec. Previously, if a buffer object was bound by glBindBufferBase(), the GL_TRANSFORM_FEEDBACK_BUFFER_SIZE query returned the size of the buffer. Fixes GLES3 conformance tests transform_feedback_{builtin_type,state_variables}.test. NAK this patch. After some more careful re-reading of the GL and GLES specs, I believe it implements the wrong behaviour. I'm thinking in particular of this text (the last two paragraphs of section 2.9.1 of the GLES 3 spec): "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 2.14.2." In the context of transform feedback, I interpret the phrase "at the time the binding is used" to mean "at the time BeginTransformFeedback is called." Mesa's current behaviour isn't consistent with the above two paragraphs: it records the size of the buffer at the time BindBufferBase is called, which means that if the buffer is grown after it's bound (but before it's used), the entire buffer won't be used. Worse yet, in the absence of extra checking by the driver, if the buffer is shrunk after it's bound, there's a danger of transform feedback results overflowing past the end of the buffer. </div></div></blockquote> I think your new interpretation is correct. Can we come up with a piglit test that would expose the buffer-growth-after-bind bug? I'm curious to see what other drivers do... </blockquote><div> </div><div>Yeah, that should be easy enough. We ought to be able to use a PRIMITIVES_WRITTEN query to detect if the driver tried to write past the end of the buffer. I'll add it to my list of things to do Monday.</div> <div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> I'm working on a replacement patch. --- src/mesa/main/get.c | 2 +- src/mesa/main/mtypes.h | 7 +++++++ src/mesa/main/transformfeedback.c | 10 ++++++---- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c index f3dbda2..2348486 100644 --- a/src/mesa/main/get.c +++ b/src/mesa/main/get.c @@ -1574,7 +1574,7 @@ 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->QuerySize[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..333b2c9 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -1827,6 +1827,13 @@ struct gl_transform_feedback_object GLintptr Offset[MAX_FEEDBACK_BUFFERS]; /** Max data to put into dest buffer (in bytes) */ GLsizeiptr Size[MAX_FEEDBACK_BUFFERS]; + /** + * Size that should be returned from GL_TRANSFORM_FEEDBACK_BUFFER_SIZE + * queries. If the transform feedback buffer was set up with + * glBindBufferBase() or glBindBufferOffsetEXT(). If it was set up with + * glBindBufferRange() it is equal to ActualSize. + */ + GLsizeiptr QuerySize[MAX_FEEDBACK_BUFFERS]; }; diff --git a/src/mesa/main/transformfeedback.c b/src/mesa/main/transformfeedback.c index 7d500bc..092c343 100644 --- a/src/mesa/main/transformfeedback.c +++ b/src/mesa/main/transformfeedback.c @@ -383,7 +383,8 @@ _mesa_EndTransformFeedback(void) static void bind_buffer_range(struct gl_context *ctx, GLuint index, struct gl_buffer_object *bufObj, - GLintptr offset, GLsizeiptr size) + GLintptr offset, GLsizeiptr size, + GLsizeiptr query_size) { struct gl_transform_feedback_object *obj = ctx->TransformFeedback.CurrentObject; @@ -407,6 +408,7 @@ bind_buffer_range(struct gl_context *ctx, GLuint index, obj->Offset[index] = offset; obj->Size[index] = size; + obj->QuerySize[index] = query_size; } @@ -450,7 +452,7 @@ _mesa_bind_buffer_range_transform_feedback(struct gl_context *ctx, return; } - bind_buffer_range(ctx, index, bufObj, offset, size); + bind_buffer_range(ctx, index, bufObj, offset, size, size); } @@ -485,7 +487,7 @@ _mesa_bind_buffer_base_transform_feedback(struct gl_context *ctx, */ size = bufObj->Size & ~0x3; - bind_buffer_range(ctx, index, bufObj, 0, size); + bind_buffer_range(ctx, index, bufObj, 0, size, 0); } @@ -546,7 +548,7 @@ _mesa_BindBufferOffsetEXT(GLenum target, GLuint index, GLuint buffer, */ size = (bufObj->Size - offset) & ~0x3; - bind_buffer_range(ctx, index, bufObj, offset, size); + bind_buffer_range(ctx, index, bufObj, offset, size, 0); } -- 1.8.0.2 </div></div> _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote> </blockquote></div> </div>