[Mesa-dev] [PATCH 13/23] mesa: hook up (Named)BufferStorageMem api

Timothy Arceri tarceri at itsqueeze.com
Fri Aug 4 09:11:57 UTC 2017


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 <andresx7 at 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.pitoiset at 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;
>>


More information about the mesa-dev mailing list