[Mesa-dev] [PATCH 08/23] main: Add entry point for NamedBufferSubData.

Martin Peres martin.peres at free.fr
Fri Feb 20 02:28:11 PST 2015


On 12/02/2015 04:05, Laura Ekstrand wrote:
> v2: review by Ian Romanick
>     - Remove "_mesa" from name of static software fallback buffer_sub_data.
>     - Remove mappedRange from _mesa_buffer_sub_data.
>     - Removed some cosmetic changes to a separate commit.
> ---
>   src/mapi/glapi/gen/ARB_direct_state_access.xml |   7 ++
>   src/mesa/main/bufferobj.c                      | 129 +++++++++++++++----------
>   src/mesa/main/bufferobj.h                      |   9 ++
>   src/mesa/main/tests/dispatch_sanity.cpp        |   1 +
>   4 files changed, 97 insertions(+), 49 deletions(-)
>
> diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml b/src/mapi/glapi/gen/ARB_direct_state_access.xml
> index 7779262..6d70b8e 100644
> --- a/src/mapi/glapi/gen/ARB_direct_state_access.xml
> +++ b/src/mapi/glapi/gen/ARB_direct_state_access.xml
> @@ -28,6 +28,13 @@
>         <param name="usage" type="GLenum" />
>      </function>
>   
> +   <function name="NamedBufferSubData" offset="assign">
> +      <param name="buffer" type="GLuint" />
> +      <param name="offset" type="GLintptr" />
> +      <param name="size" type="GLsizeiptr" />
> +      <param name="data" type="const GLvoid *" />
> +   </function>
> +
>      <!-- Texture object functions -->
>   
>      <function name="CreateTextures" offset="assign">
> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
> index ac8eed1..4f89748 100644
> --- a/src/mesa/main/bufferobj.c
> +++ b/src/mesa/main/bufferobj.c
> @@ -227,67 +227,58 @@ bufferobj_range_mapped(const struct gl_buffer_object *obj,
>    * \c glClearBufferSubData.
>    *
>    * \param ctx     GL context.
> - * \param target  Buffer object target on which to operate.
> + * \param bufObj  The buffer object.
>    * \param offset  Offset of the first byte of the subdata range.
>    * \param size    Size, in bytes, of the subdata range.
>    * \param mappedRange  If true, checks if an overlapping range is mapped.
>    *                     If false, checks if buffer is mapped.
> - * \param errorNoBuffer  Error code if no buffer is bound to target.
>    * \param caller  Name of calling function for recording errors.
> - * \return   A pointer to the buffer object bound to \c target in the
> - *           specified context or \c NULL if any of the parameter or state
> - *           conditions are invalid.
> + * \return   false if error, true otherwise
>    *
>    * \sa glBufferSubDataARB, glGetBufferSubDataARB, glClearBufferSubData
>    */
> -static struct gl_buffer_object *
> -buffer_object_subdata_range_good(struct gl_context * ctx, GLenum target,
> -                                 GLintptrARB offset, GLsizeiptrARB size,
> -                                 bool mappedRange, GLenum errorNoBuffer,
> -                                 const char *caller)
> +static bool
> +buffer_object_subdata_range_good(struct gl_context *ctx,
> +                                 struct gl_buffer_object *bufObj,
> +                                 GLintptr offset, GLsizeiptr size,
> +                                 bool mappedRange, const char *caller)
>   {
> -   struct gl_buffer_object *bufObj;
> -
>      if (size < 0) {
>         _mesa_error(ctx, GL_INVALID_VALUE, "%s(size < 0)", caller);
> -      return NULL;
> +      return false;
>      }
>   
>      if (offset < 0) {
>         _mesa_error(ctx, GL_INVALID_VALUE, "%s(offset < 0)", caller);
> -      return NULL;
> +      return false;
>      }
>   
> -   bufObj = get_buffer(ctx, caller, target, errorNoBuffer);
> -   if (!bufObj)
> -      return NULL;
> -
>      if (offset + size > bufObj->Size) {
>         _mesa_error(ctx, GL_INVALID_VALUE,
>                     "%s(offset %lu + size %lu > buffer size %lu)", caller,
>                     (unsigned long) offset,
>                     (unsigned long) size,
>                     (unsigned long) bufObj->Size);
> -      return NULL;
> +      return false;
>      }
>   
>      if (bufObj->Mappings[MAP_USER].AccessFlags & GL_MAP_PERSISTENT_BIT)
> -      return bufObj;
> +      return true;
>   
>      if (mappedRange) {
>         if (bufferobj_range_mapped(bufObj, offset, size)) {
>            _mesa_error(ctx, GL_INVALID_OPERATION, "%s", caller);
> -         return NULL;
> +         return false;
>         }
>      }
>      else {
>         if (_mesa_bufferobj_mapped(bufObj, MAP_USER)) {
>            _mesa_error(ctx, GL_INVALID_OPERATION, "%s", caller);
> -         return NULL;
> +         return false;
>         }
>      }
>   
> -   return bufObj;
> +   return true;
>   }
>   
>   
> @@ -602,9 +593,9 @@ buffer_data_fallback(struct gl_context *ctx, GLenum target, GLsizeiptr size,
>    * \sa glBufferSubDataARB, dd_function_table::BufferSubData.
>    */
>   static void
> -_mesa_buffer_subdata( struct gl_context *ctx, GLintptrARB offset,
> -		      GLsizeiptrARB size, const GLvoid * data,
> -		      struct gl_buffer_object * bufObj )
> +buffer_sub_data_fallback(struct gl_context *ctx, GLintptr offset,
> +                         GLsizeiptr size, const GLvoid *data,
> +                         struct gl_buffer_object *bufObj)
>   {
>      (void) ctx;
>   
> @@ -1113,7 +1104,7 @@ _mesa_init_buffer_object_functions(struct dd_function_table *driver)
>      driver->NewBufferObject = _mesa_new_buffer_object;
>      driver->DeleteBuffer = _mesa_delete_buffer_object;
>      driver->BufferData = buffer_data_fallback;
> -   driver->BufferSubData = _mesa_buffer_subdata;
> +   driver->BufferSubData = buffer_sub_data_fallback;
>      driver->GetBufferSubData = _mesa_buffer_get_subdata;
>      driver->UnmapBuffer = _mesa_buffer_unmap;
>   
> @@ -1588,24 +1579,31 @@ _mesa_NamedBufferData(GLuint buffer, GLsizeiptr size, const GLvoid *data,
>   }
>   
>   
> -void GLAPIENTRY
> -_mesa_BufferSubData(GLenum target, GLintptrARB offset,
> -                       GLsizeiptrARB size, const GLvoid * data)
> +/**
> + * Implementation for glBufferSubData and glNamedBufferSubData.
> + *
> + * \param ctx     GL context.
> + * \param bufObj  The buffer object.
> + * \param offset  Offset of the first byte of the subdata range.
> + * \param size    Size, in bytes, of the subdata range.
> + * \param data    The data store.
> + * \param func  Name of calling function for recording errors.
> + *
> + */
> +void
> +_mesa_buffer_sub_data(struct gl_context *ctx, struct gl_buffer_object *bufObj,
> +                      GLintptr offset, GLsizeiptr size, const GLvoid *data,
> +                      const char *func)
>   {
> -   GET_CURRENT_CONTEXT(ctx);
> -   struct gl_buffer_object *bufObj;
> -
> -   bufObj = buffer_object_subdata_range_good( ctx, target, offset, size,
> -                                              false, GL_INVALID_OPERATION,
> -                                              "glBufferSubDataARB" );
> -   if (!bufObj) {
> +   if (!buffer_object_subdata_range_good(ctx, bufObj, offset, size,
> +                                         false, func)) {
>         /* error already recorded */
>         return;
>      }
>   
>      if (bufObj->Immutable &&
>          !(bufObj->StorageFlags & GL_DYNAMIC_STORAGE_BIT)) {
> -      _mesa_error(ctx, GL_INVALID_OPERATION, "glBufferSubData");
> +      _mesa_error(ctx, GL_INVALID_OPERATION, func);
>         return;
>      }
>   
> @@ -1618,19 +1616,50 @@ _mesa_BufferSubData(GLenum target, GLintptrARB offset,
>      ctx->Driver.BufferSubData( ctx, offset, size, data, bufObj );
>   }
>   
> +void GLAPIENTRY
> +_mesa_BufferSubData(GLenum target, GLintptr offset,
> +                    GLsizeiptr size, const GLvoid *data)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +   struct gl_buffer_object *bufObj;
> +
> +   bufObj = get_buffer(ctx, "glBufferSubData", target, GL_INVALID_OPERATION);
> +   if (!bufObj)
> +      return;
> +
> +   _mesa_buffer_sub_data(ctx, bufObj, offset, size, data, "glBufferSubData");
> +}
>   
>   void GLAPIENTRY
> -_mesa_GetBufferSubData(GLenum target, GLintptrARB offset,
> -                          GLsizeiptrARB size, void * data)
> +_mesa_NamedBufferSubData(GLuint buffer, GLintptr offset,
> +                         GLsizeiptr size, const GLvoid *data)

In this case, I would have added _mesa_NamedBufferSubData under
_mesa_GetBufferSubData to make the diff more readable. It is good
practice to make the diff as short as possible and to prove that you
changed as little as possible in the existing functions :)

Other than that, looks good to me!

Reviewed-by: Martin Peres <martin.peres at linux.intel.com>

>   {
>      GET_CURRENT_CONTEXT(ctx);
>      struct gl_buffer_object *bufObj;
>   
> -   bufObj = buffer_object_subdata_range_good(ctx, target, offset, size,
> -                                             false, GL_INVALID_OPERATION,
> -                                             "glGetBufferSubDataARB");
> -   if (!bufObj) {
> -      /* error already recorded */
> +   bufObj = _mesa_lookup_bufferobj_err(ctx, buffer, "glNamedBufferSubData");
> +   if (!bufObj)
> +      return;
> +
> +   _mesa_buffer_sub_data(ctx, bufObj, offset, size, data,
> +                         "glNamedBufferSubData");
> +}
> +
> +
> +void GLAPIENTRY
> +_mesa_GetBufferSubData(GLenum target, GLintptr offset,
> +                       GLsizeiptr size, GLvoid *data)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +   struct gl_buffer_object *bufObj;
> +
> +   bufObj = get_buffer(ctx, "glGetBufferSubData", target,
> +                       GL_INVALID_OPERATION);
> +   if (!bufObj)
> +      return;
> +
> +   if (!buffer_object_subdata_range_good(ctx, bufObj, offset, size, false,
> +                                         "glGetBufferSubData")) {
>         return;
>      }
>   
> @@ -1704,10 +1733,12 @@ _mesa_ClearBufferSubData(GLenum target, GLenum internalformat,
>      GLubyte clearValue[MAX_PIXEL_BYTES];
>      GLsizeiptr clearValueSize;
>   
> -   bufObj = buffer_object_subdata_range_good(ctx, target, offset, size,
> -                                             true, GL_INVALID_VALUE,
> -                                             "glClearBufferSubData");
> -   if (!bufObj) {
> +   bufObj = get_buffer(ctx, "glClearBufferSubData", target, GL_INVALID_VALUE);
> +   if (!bufObj)
> +      return;
> +
> +   if (!buffer_object_subdata_range_good(ctx, bufObj, offset, size,
> +                                         true, "glClearBufferSubData")) {
>         return;
>      }
>   
> diff --git a/src/mesa/main/bufferobj.h b/src/mesa/main/bufferobj.h
> index ddd240c..889bbb1 100644
> --- a/src/mesa/main/bufferobj.h
> +++ b/src/mesa/main/bufferobj.h
> @@ -140,6 +140,11 @@ _mesa_buffer_data(struct gl_context *ctx, struct gl_buffer_object *bufObj,
>                     GLenum usage, const char *func);
>   
>   extern void
> +_mesa_buffer_sub_data(struct gl_context *ctx, struct gl_buffer_object *bufObj,
> +                      GLintptr offset, GLsizeiptr size, const GLvoid *data,
> +                      const char *func);
> +
> +extern void
>   _mesa_buffer_unmap_all_mappings(struct gl_context *ctx,
>                                   struct gl_buffer_object *bufObj);
>   
> @@ -189,6 +194,10 @@ _mesa_BufferSubData(GLenum target, GLintptrARB offset,
>                       GLsizeiptrARB size, const GLvoid * data);
>   
>   void GLAPIENTRY
> +_mesa_NamedBufferSubData(GLuint buffer, GLintptr offset,
> +                         GLsizeiptr size, const GLvoid *data);
> +
> +void GLAPIENTRY
>   _mesa_GetBufferSubData(GLenum target, GLintptrARB offset,
>                          GLsizeiptrARB size, void * data);
>   
> diff --git a/src/mesa/main/tests/dispatch_sanity.cpp b/src/mesa/main/tests/dispatch_sanity.cpp
> index 595ee90..898a2b9 100644
> --- a/src/mesa/main/tests/dispatch_sanity.cpp
> +++ b/src/mesa/main/tests/dispatch_sanity.cpp
> @@ -958,6 +958,7 @@ const struct function gl_core_functions_possible[] = {
>      { "glCreateBuffers", 45, -1 },
>      { "glNamedBufferStorage", 45, -1 },
>      { "glNamedBufferData", 45, -1 },
> +   { "glNamedBufferSubData", 45, -1 },
>      { "glCreateTextures", 45, -1 },
>      { "glTextureStorage1D", 45, -1 },
>      { "glTextureStorage2D", 45, -1 },



More information about the mesa-dev mailing list