On 04/08/17 18:49, Samuel Pitoiset wrote:


On 08/04/2017 03:54 AM, Timothy Arceri wrote:
Include no_error variants as well.

v2 (Timothy Arceri):
  - reduced code churn by squashing some changes into
    previous commits

v3 (Timothy Arceri):
  - drop unused function declaration

v4 (Timothy Arceri):
  - fix Driver function assert()
  - add missing GL errors

Signed-off-by: Andres Rodriguez <andre...@gmail.com>
---
src/mesa/main/bufferobj.c | 86 ++++++++++++++++++++++++++++++++++++-----------
  1 file changed, 66 insertions(+), 20 deletions(-)

diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
index 05c15661b2..6c5943d3b9 100644
--- a/src/mesa/main/bufferobj.c
+++ b/src/mesa/main/bufferobj.c
@@ -32,20 +32,21 @@
  #include <stdbool.h>
  #include <inttypes.h>  /* for PRId64 macro */
  #include "util/debug.h"
  #include "glheader.h"
  #include "enums.h"
  #include "hash.h"
  #include "imports.h"
  #include "context.h"
  #include "bufferobj.h"
+#include "externalobjects.h"
  #include "mtypes.h"
  #include "teximage.h"
  #include "glformats.h"
  #include "texstore.h"
  #include "transformfeedback.h"
  #include "varray.h"
  /* Debug flags */
  /*#define VBO_DEBUG*/
@@ -1813,56 +1814,97 @@ validate_buffer_storage(struct gl_context *ctx,
        _mesa_error(ctx, GL_INVALID_OPERATION, "%s(immutable)", func);
        return false;
     }
     return true;
  }
  static void
  buffer_storage(struct gl_context *ctx, struct gl_buffer_object *bufObj,
-               GLenum target, GLsizeiptr size, const GLvoid *data,
-               GLbitfield flags, const char *func)
+               struct gl_memory_object *memObj, GLenum target,
+               GLsizeiptr size, const GLvoid *data, GLbitfield flags,
+               GLuint64 offset, const char *func)
  {
+   GLboolean err;
+
/* Unmap the existing buffer. We'll replace it now. Not an error. */
     _mesa_buffer_unmap_all_mappings(ctx, bufObj);
     FLUSH_VERTICES(ctx, 0);
     bufObj->Written = GL_TRUE;
     bufObj->Immutable = GL_TRUE;
     bufObj->MinMaxCacheDirty = true;
-   assert(ctx->Driver.BufferData);
-   if (!ctx->Driver.BufferData(ctx, target, size, data, GL_DYNAMIC_DRAW,
-                               flags, bufObj)) {
+   if (memObj) {
+      assert(ctx->Driver.BufferDataMem);
+      err = ctx->Driver.BufferDataMem(ctx, target, size, memObj, offset,
+                                      GL_DYNAMIC_DRAW, bufObj);
+   }
+   else {
+      assert(ctx->Driver.BufferData);
+ err = ctx->Driver.BufferData(ctx, target, size, data, GL_DYNAMIC_DRAW,
+                                   flags, bufObj);
+   }
+
+   if (err) {
        if (target == GL_EXTERNAL_VIRTUAL_MEMORY_BUFFER_AMD) {
           /* Even though the interaction between AMD_pinned_memory and
* glBufferStorage is not described in the spec, Graham Sellers
            * said that it should behave the same as glBufferData.
            */
           _mesa_error(ctx, GL_INVALID_OPERATION, "%s", func);
        }
        else {
           _mesa_error(ctx, GL_OUT_OF_MEMORY, "%s", func);
        }
     }
  }
  static ALWAYS_INLINE void
  inlined_buffer_storage(GLenum target, GLuint buffer, GLsizeiptr size,
-                       const GLvoid *data, GLbitfield flags, bool dsa,
-                       bool no_error, const char *func)
+                       const GLvoid *data, GLbitfield flags,
+                       GLuint memory, GLuint64 offset,
+ bool dsa, bool mem, bool no_error, const char *func)
  {
     GET_CURRENT_CONTEXT(ctx);
     struct gl_buffer_object *bufObj;
+   struct gl_memory_object *memObj = NULL;
+
+   if (mem) {
+      /* From the EXT_external_objects spec:
+       *
+ * "An INVALID_VALUE error is generated by BufferStorageMemEXT and + * NamedBufferStorageMemEXT if <memory> is 0, or if <offset> + <size>
+       *   is greater than the size of the specified memory object."

Because you don't check the second condition yet (maybe add a TODO?), I think you can remove it.

All offset checks are implementation specific as far as I understand. Yeah I'll remove the last part of the quote.


With this fixed, patch is:

Reviewed-by: Samuel Pitoiset <samuel.pitoi...@gmail.com>

+       */
+      if (!no_error && memory == 0) {
+         _mesa_error(ctx, GL_INVALID_VALUE, "%s(memory == 0)", func);
+      }
+
+      memObj = _mesa_lookup_memory_object(ctx, memory);
+      if (!memObj)
+         return;
+
+      /* From the EXT_external_objects spec:
+       *
+       *   "An INVALID_OPERATION error is generated if <memory> names a
+       *   valid memory object which has no associated memory."
+       */
+      if (!no_error && !memObj->Immutable) {
+ _mesa_error(ctx, GL_INVALID_OPERATION, "%s(no associated memory)",
+                     func);
+         return;
+      }
+   }
     if (dsa) {
        if (no_error) {
           bufObj = _mesa_lookup_bufferobj(ctx, buffer);
        } else {
           bufObj = _mesa_lookup_bufferobj_err(ctx, buffer, func);
           if (!bufObj)
              return;
        }
     } else {
@@ -1870,94 +1912,98 @@ inlined_buffer_storage(GLenum target, GLuint buffer, GLsizeiptr size, struct gl_buffer_object **bufObjPtr = get_buffer_target(ctx, target);
           bufObj = *bufObjPtr;
        } else {
           bufObj = get_buffer(ctx, func, target, GL_INVALID_OPERATION);
           if (!bufObj)
              return;
        }
     }
if (no_error || validate_buffer_storage(ctx, bufObj, size, flags, func))
-      buffer_storage(ctx, bufObj, target, size, data, flags, func);
+ buffer_storage(ctx, bufObj, memObj, target, size, data, flags, offset, func);
  }
  void GLAPIENTRY
  _mesa_BufferStorage_no_error(GLenum target, GLsizeiptr size,
                               const GLvoid *data, GLbitfield flags)
  {
-   inlined_buffer_storage(target, 0, size, data, flags, false, true,
-                          "glBufferStorage");
+   inlined_buffer_storage(target, 0, size, data, flags, GL_NONE, 0,
+                          false, false, true, "glBufferStorage");
  }
  void GLAPIENTRY
  _mesa_BufferStorage(GLenum target, GLsizeiptr size, const GLvoid *data,
                      GLbitfield flags)
  {
-   inlined_buffer_storage(target, 0, size, data, flags, false, false,
-                          "glBufferStorage");
+   inlined_buffer_storage(target, 0, size, data, flags, GL_NONE, 0,
+                          false, false, false, "glBufferStorage");
  }
  void GLAPIENTRY
  _mesa_BufferStorageMemEXT(GLenum target, GLsizeiptr size,
                            GLuint memory, GLuint64 offset)
  {
-
+   inlined_buffer_storage(target, 0, size, NULL, 0, memory, offset,
+                          false, true, false, "glBufferStorageMemEXT");
  }
  void GLAPIENTRY
  _mesa_BufferStorageMemEXT_no_error(GLenum target, GLsizeiptr size,
                                     GLuint memory, GLuint64 offset)
  {
-
+   inlined_buffer_storage(target, 0, size, NULL, 0, memory, offset,
+                          false, true, true, "glBufferStorageMemEXT");
  }
  void GLAPIENTRY
  _mesa_NamedBufferStorage_no_error(GLuint buffer, GLsizeiptr size,
                                    const GLvoid *data, GLbitfield flags)
  {
     /* In direct state access, buffer objects have an unspecified target
      * since they are not required to be bound.
      */
- inlined_buffer_storage(GL_NONE, buffer, size, data, flags, true, true,
-                          "glNamedBufferStorage");
+ inlined_buffer_storage(GL_NONE, buffer, size, data, flags, GL_NONE, 0,
+                          true, false, true, "glNamedBufferStorage");
  }
  void GLAPIENTRY
_mesa_NamedBufferStorage(GLuint buffer, GLsizeiptr size, const GLvoid *data,
                           GLbitfield flags)
  {
     /* In direct state access, buffer objects have an unspecified target
      * since they are not required to be bound.
      */
- inlined_buffer_storage(GL_NONE, buffer, size, data, flags, true, false,
-                          "glNamedBufferStorage");
+ inlined_buffer_storage(GL_NONE, buffer, size, data, flags, GL_NONE, 0,
+                          true, false, false, "glNamedBufferStorage");
  }
  void GLAPIENTRY
  _mesa_NamedBufferStorageMemEXT(GLuint buffer, GLsizeiptr size,
                                 GLuint memory, GLuint64 offset)
  {
-
+ inlined_buffer_storage(GL_NONE, buffer, size, GL_NONE, 0, memory, offset, + true, true, false, "glNamedBufferStorageMemEXT");
  }
  void GLAPIENTRY
  _mesa_NamedBufferStorageMemEXT_no_error(GLuint buffer, GLsizeiptr size,
                                          GLuint memory, GLuint64 offset)
  {
-
+ inlined_buffer_storage(GL_NONE, buffer, size, GL_NONE, 0, memory, offset, + true, true, true, "glNamedBufferStorageMemEXT");
  }
  static ALWAYS_INLINE void
  buffer_data(struct gl_context *ctx, struct gl_buffer_object *bufObj,
GLenum target, GLsizeiptr size, const GLvoid *data, GLenum usage,
              const char *func, bool no_error)
  {
     bool valid_usage;

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to