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

Laura Ekstrand laura at jlekstrand.net
Fri Feb 20 10:03:44 PST 2015


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 :)

On Fri, Feb 20, 2015 at 6:18 AM, Martin Peres <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>
>
>  +                            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;
>>      }
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150220/55ab8861/attachment-0001.html>


More information about the mesa-dev mailing list