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 <jordan.l.justen@intel.com>

diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
index f3dbda2d3d2259f0b7959c9e950eab88f6104e20..273a79f7fb7240a4d3a4b35784c01ec7cfd1abc8 100644 (file)
--- 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 0c201d08f460ce243af64e612d522d9ece1d0a19..097cdc14f58cb98328e4fd98da88a3e0573b8353 100644 (file)
--- 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 b2818caa591623854ae924c1256ff267314c3f64..61f2f4f11d01de8776625c7921e44dbdc21a6a28 100644 (file)
--- 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);
 }