[Mesa-dev] [PATCH 11/23] main: Refactor ClearBuffer[Sub]Data.

Martin Peres martin.peres at free.fr
Fri Feb 20 13:06:14 PST 2015


On 20/02/2015 20:03, Laura Ekstrand wrote:
> This naming convention tries to match the corresponding DD table entry.
>
> There's a thread discussing the naming convention for external 
> software fallback functions.  Feel free to add your input to the 
> discussion there :)

Ack, no strong opinion here :)

>
> On Fri, Feb 20, 2015 at 6:18 AM, Martin Peres <martin.peres at free.fr 
> <mailto:martin.peres at free.fr>> wrote:
>
>     On 12/02/2015 04:05, Laura Ekstrand wrote:
>
>         v2: review by Jason Ekstrand
>             - Split refactor of clear buffer sub data from addition of
>         DSA entry
>               points.
>         ---
>           src/mesa/main/bufferobj.c                    | 125
>         ++++++++++++---------------
>           src/mesa/main/bufferobj.h                    |  19 ++--
>           src/mesa/state_tracker/st_cb_bufferobjects.c |   4 +-
>           3 files changed, 69 insertions(+), 79 deletions(-)
>
>         diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
>         index 7225b64..b8fa917 100644
>         --- a/src/mesa/main/bufferobj.c
>         +++ b/src/mesa/main/bufferobj.c
>         @@ -660,11 +660,11 @@ _mesa_buffer_get_subdata( struct
>         gl_context *ctx, GLintptrARB offset,
>            * dd_function_table::ClearBufferSubData.
>            */
>           void
>         -_mesa_buffer_clear_subdata(struct gl_context *ctx,
>         -                           GLintptr offset, GLsizeiptr size,
>         -                           const GLvoid *clearValue,
>         -                           GLsizeiptr clearValueSize,
>         -                           struct gl_buffer_object *bufObj)
>         +_mesa_ClearBufferSubData_sw(struct gl_context *ctx,
>
>
>     Interesting choice of naming the function as a mix of camel case
>     and underscores.
>
>     Since it is an internal function, shouldn't it only have underscores?
>
>     Other than that:
>
>     Reviewed-by: Martin Peres <martin.peres at linux.intel.com
>     <mailto:martin.peres at linux.intel.com>>
>
>         +                            GLintptr offset, GLsizeiptr size,
>         +                            const GLvoid *clearValue,
>         +                            GLsizeiptr clearValueSize,
>         +                            struct gl_buffer_object *bufObj)
>           {
>              GLsizeiptr i;
>              GLubyte *dest;
>         @@ -1113,7 +1113,7 @@
>         _mesa_init_buffer_object_functions(struct dd_function_table
>         *driver)
>              driver->UnmapBuffer = _mesa_buffer_unmap;
>                /* GL_ARB_clear_buffer_object */
>         -   driver->ClearBufferSubData = _mesa_buffer_clear_subdata;
>         +   driver->ClearBufferSubData = _mesa_ClearBufferSubData_sw;
>                /* GL_ARB_map_buffer_range */
>              driver->MapBufferRange = _mesa_buffer_map_range;
>         @@ -1671,57 +1671,77 @@ _mesa_GetBufferSubData(GLenum target,
>         GLintptr offset,
>              ctx->Driver.GetBufferSubData( ctx, offset, size, data,
>         bufObj );
>           }
>           -
>         -void GLAPIENTRY
>         -_mesa_ClearBufferData(GLenum target, GLenum internalformat,
>         GLenum format,
>         -                      GLenum type, const GLvoid* data)
>         +/**
>         + * \param subdata   true if caller is *SubData, false if *Data
>         + */
>         +void
>         +_mesa_clear_buffer_sub_data(struct gl_context *ctx,
>         +                            struct gl_buffer_object *bufObj,
>         +                            GLenum internalformat,
>         +                            GLintptr offset, GLsizeiptr size,
>         +                            GLenum format, GLenum type,
>         +                            const GLvoid *data,
>         +                            const char *func, bool subdata)
>           {
>         -   GET_CURRENT_CONTEXT(ctx);
>         -   struct gl_buffer_object* bufObj;
>              mesa_format mesaFormat;
>              GLubyte clearValue[MAX_PIXEL_BYTES];
>              GLsizeiptr clearValueSize;
>           -   bufObj = get_buffer(ctx, "glClearBufferData", target,
>         GL_INVALID_VALUE);
>         -   if (!bufObj) {
>         -      return;
>         -   }
>         -
>         -   if (_mesa_check_disallowed_mapping(bufObj)) {
>         -      _mesa_error(ctx, GL_INVALID_OPERATION,
>         -                  "glClearBufferData(buffer currently mapped)");
>         +   /* This checks for disallowed mappings. */
>         +   if (!buffer_object_subdata_range_good(ctx, bufObj, offset,
>         size,
>         +                                         subdata, func)) {
>                 return;
>              }
>                mesaFormat = validate_clear_buffer_format(ctx,
>         internalformat,
>         -                                             format, type,
>         -  "glClearBufferData");
>         +                                             format, type, func);
>         +
>              if (mesaFormat == MESA_FORMAT_NONE) {
>                 return;
>              }
>                clearValueSize = _mesa_get_format_bytes(mesaFormat);
>         -   if (bufObj->Size % clearValueSize != 0) {
>         +   if (offset % clearValueSize != 0 || size % clearValueSize
>         != 0) {
>                 _mesa_error(ctx, GL_INVALID_VALUE,
>         -                  "glClearBufferData(size is not a multiple of "
>         -                  "internalformat size)");
>         +                  "%s(offset or size is not a multiple of "
>         +                  "internalformat size)", func);
>                 return;
>              }
>                if (data == NULL) {
>                 /* clear to zeros, per the spec */
>         -      ctx->Driver.ClearBufferSubData(ctx, 0, bufObj->Size,
>         -                                     NULL, clearValueSize,
>         bufObj);
>         +      if (size > 0) {
>         +         ctx->Driver.ClearBufferSubData(ctx, offset, size,
>         +                                        NULL, clearValueSize,
>         bufObj);
>         +      }
>                 return;
>              }
>                if (!convert_clear_buffer_data(ctx, mesaFormat, clearValue,
>         -                                  format, type, data,
>         "glClearBufferData")) {
>         +                                  format, type, data, func)) {
>                 return;
>              }
>           -   ctx->Driver.ClearBufferSubData(ctx, 0, bufObj->Size,
>         -                                  clearValue, clearValueSize,
>         bufObj);
>         +   if (size > 0) {
>         +      ctx->Driver.ClearBufferSubData(ctx, offset, size,
>         +                                     clearValue,
>         clearValueSize, bufObj);
>         +   }
>         +}
>         +
>         +void GLAPIENTRY
>         +_mesa_ClearBufferData(GLenum target, GLenum internalformat,
>         GLenum format,
>         +                      GLenum type, const GLvoid *data)
>         +{
>         +   GET_CURRENT_CONTEXT(ctx);
>         +   struct gl_buffer_object *bufObj;
>         +
>         +   bufObj = get_buffer(ctx, "glClearBufferData", target,
>         GL_INVALID_VALUE);
>         +   if (!bufObj)
>         +      return;
>         +
>         +   _mesa_clear_buffer_sub_data(ctx, bufObj, internalformat,
>         0, bufObj->Size,
>         +                               format, type, data,
>         +                               "glClearBufferData", false);
>           }
>             @@ -1733,53 +1753,14 @@ _mesa_ClearBufferSubData(GLenum
>         target, GLenum internalformat,
>           {
>              GET_CURRENT_CONTEXT(ctx);
>              struct gl_buffer_object* bufObj;
>         -   mesa_format mesaFormat;
>         -   GLubyte clearValue[MAX_PIXEL_BYTES];
>         -   GLsizeiptr clearValueSize;
>                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;
>         -   }
>         -
>         -   mesaFormat = validate_clear_buffer_format(ctx, internalformat,
>         -                                             format, type,
>         -  "glClearBufferSubData");
>         -   if (mesaFormat == MESA_FORMAT_NONE) {
>         -      return;
>         -   }
>         -
>         -   clearValueSize = _mesa_get_format_bytes(mesaFormat);
>         -   if (offset % clearValueSize != 0 || size % clearValueSize
>         != 0) {
>         -      _mesa_error(ctx, GL_INVALID_VALUE,
>         -                  "glClearBufferSubData(offset or size is not
>         a multiple of "
>         -                  "internalformat size)");
>         -      return;
>         -   }
>         -
>         -   if (data == NULL) {
>         -      /* clear to zeros, per the spec */
>         -      if (size > 0) {
>         -         ctx->Driver.ClearBufferSubData(ctx, offset, size,
>         -                                        NULL, clearValueSize,
>         bufObj);
>         -      }
>         -      return;
>         -   }
>         -
>         -   if (!convert_clear_buffer_data(ctx, mesaFormat, clearValue,
>         -                                  format, type, data,
>         - "glClearBufferSubData")) {
>         -      return;
>         -   }
>         -
>         -   if (size > 0) {
>         -      ctx->Driver.ClearBufferSubData(ctx, offset, size,
>         -                                     clearValue,
>         clearValueSize, bufObj);
>         -   }
>         +   _mesa_clear_buffer_sub_data(ctx, bufObj, internalformat,
>         offset, size,
>         +                               format, type, data,
>         +  "glClearBufferSubData", true);
>           }
>             diff --git a/src/mesa/main/bufferobj.h
>         b/src/mesa/main/bufferobj.h
>         index 7db5c98..5911270 100644
>         --- a/src/mesa/main/bufferobj.h
>         +++ b/src/mesa/main/bufferobj.h
>         @@ -156,11 +156,20 @@ _mesa_copy_buffer_sub_data(struct
>         gl_context *ctx,
>                                      GLsizeiptr size, const char *func);
>             extern void
>         -_mesa_buffer_clear_subdata(struct gl_context *ctx,
>         -                           GLintptr offset, GLsizeiptr size,
>         -                           const GLvoid *clearValue,
>         -                           GLsizeiptr clearValueSize,
>         -                           struct gl_buffer_object *bufObj);
>         +_mesa_ClearBufferSubData_sw(struct gl_context *ctx,
>         +                            GLintptr offset, GLsizeiptr size,
>         +                            const GLvoid *clearValue,
>         +                            GLsizeiptr clearValueSize,
>         +                            struct gl_buffer_object *bufObj);
>         +
>         +extern void
>         +_mesa_clear_buffer_sub_data(struct gl_context *ctx,
>         +                            struct gl_buffer_object *bufObj,
>         +                            GLenum internalformat,
>         +                            GLintptr offset, GLsizeiptr size,
>         +                            GLenum format, GLenum type,
>         +                            const GLvoid *data,
>         +                            const char *func, bool subdata);
>             /*
>            * API functions
>         diff --git a/src/mesa/state_tracker/st_cb_bufferobjects.c
>         b/src/mesa/state_tracker/st_cb_bufferobjects.c
>         index 90f786c..b7096f7 100644
>         --- a/src/mesa/state_tracker/st_cb_bufferobjects.c
>         +++ b/src/mesa/state_tracker/st_cb_bufferobjects.c
>         @@ -477,8 +477,8 @@ st_clear_buffer_subdata(struct gl_context
>         *ctx,
>              static const char zeros[16] = {0};
>                if (!pipe->clear_buffer) {
>         -      _mesa_buffer_clear_subdata(ctx, offset, size,
>         -                                 clearValue, clearValueSize,
>         bufObj);
>         +      _mesa_ClearBufferSubData_sw(ctx, offset, size,
>         +                                  clearValue, clearValueSize,
>         bufObj);
>                 return;
>              }
>
>
>



More information about the mesa-dev mailing list