[Mesa-dev] [PATCH V2 07/10] mesa: Implement functions for clear_buffer_object extensions

[Pi Tabred]

On 12.12.2013 01:39, Brian Paul wrote:
> On 12/11/2013 02:55 PM, Pi Tabred wrote:
>>   - _mesa_buffer_clear_subdata: default callback for dd function table
>>   - _mesa_ClearBufferData: API function
>>   - _mesa_ClearBufferSubData: API function
>>   - buffer_object_format_good: helper function, check if the
>> internalformat,
>>     format and type parameter are legal
>>   - buffer_object_convert_clear: helper function, convert the supplied
>> data
>>     to the desired internalformat and clear the buffer by calling the
>>     callback for dd_function_table::ClearbufferSubData
>> ---
>>   src/mesa/main/bufferobj.c | 250
>> ++++++++++++++++++++++++++++++++++++++++++++++
>>   src/mesa/main/bufferobj.h |   4 +
>>   2 files changed, 254 insertions(+)
>>
>> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
>> index 0e5b705..72515ef 100644
>> --- a/src/mesa/main/bufferobj.c
>> +++ b/src/mesa/main/bufferobj.c
>> @@ -41,6 +41,9 @@
>>   #include "fbobject.h"
>>   #include "mtypes.h"
>>   #include "texobj.h"
>> +#include "teximage.h"
>> +#include "glformats.h"
>> +#include "texstore.h"
>>   #include "transformfeedback.h"
>>   #include "dispatch.h"
>>
>> @@ -283,6 +286,120 @@ buffer_object_subdata_range_good(struct
>> gl_context * ctx, GLenum target,
>>
>>
>>   /**
>> + * Tests the format and type parameters and sets the GL error code for
>> + * \c glClearBufferData and \c glClearBufferSubData.
>> + *
>> + * \param ctx            GL context.
>> + * \param target         Buffer object target on which to operate.
>> + * \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.
> 
> But the code below returns a gl_format, not a pointer.
> 

sorry about that, to much copy-paste.

>> + *
>> + * \sa glBufferSubDataARB, glGetBufferSubDataARB, glClearBufferSubData
>> + */
>> +static gl_format
>> +buffer_object_format_good(struct gl_context *ctx,
>> +                          const struct gl_buffer_object *obj,
>> +                          GLenum internalformat, GLenum format,
>> GLenum type,
>> +                          const char* caller)
> 
> Let's rename this function to something like
> "validate_buffer_clear_format".
> 
> 

sure, I just tried to be in line with the naming of the
buffer_object_subdata_range_good function

>> +{
>> +   gl_format internalFormatMesa;
> 
> I think mesaFormat would be more concise.
> 
> 
>> +   GLenum errorFormatType;
>> +
>> +   internalFormatMesa = _mesa_validate_texbuffer_format(ctx,
>> internalformat);
>> +   if (internalFormatMesa == MESA_FORMAT_NONE) {
>> +      _mesa_error(ctx, GL_INVALID_ENUM,
>> +                  "%s(invalid internalformat)", caller);
>> +      return MESA_FORMAT_NONE;
>> +   }
>> +
>> +   /* NOTE: not mentioned in ARB_clear_buffer_object but according to
>> +    * EXT_texture_integer there is no conversion between integer and
>> +    * non-integer formats
>> +   */
>> +   if (_mesa_is_enum_format_signed_int(format) !=
>> +       _mesa_is_format_integer_color(internalFormatMesa)) {
>> +      _mesa_error(ctx, GL_INVALID_OPERATION,
>> +                  "%s(integer vs non-integer)", caller);
>> +      return MESA_FORMAT_NONE;
>> +   }
>> +
>> +   if (!_mesa_is_color_format(format)) {
>> +      _mesa_error(ctx, GL_INVALID_ENUM,
>> +                  "%s(format is not a color format)", caller);
>> +      return MESA_FORMAT_NONE;
>> +   }
>> +
>> +   errorFormatType = _mesa_error_check_format_and_type(ctx, format,
>> +                                                       type);
>> +   if (errorFormatType != GL_NO_ERROR) {
>> +      _mesa_error(ctx, GL_INVALID_ENUM,
>> +                  "%s(invalid format or type)", caller);
>> +      return MESA_FORMAT_NONE;
>> +   }
>> +
>> +   return internalFormatMesa;
>> +}
>> +
>> +
>> +/**
>> + * Converts the supplied data to the internalformat and clears the
>> desired
>> + * range.
>> + *
>> + * \param ctx             GL context.
>> + * \param offset          Offset of the to be cleared range.
>> + * \param size            Size of the to be cleared range.
>> + * \param internalformat  Format to which the data is converted.
>> + * \param sizeOfFormat    Size of the internalformat in bytes.
>> + * \param format          Format of the supplied data.
>> + * \param type            Type of the supplied data.
>> + * \param data            Data which is used to clear the buffer.
>> + * \param bufObj          To be cleared buffer.
>> + * \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.
>> + *
>> + * \sa glClearBufferData, glClearBufferSubData
>> + */
>> +static void
>> +buffer_object_convert_clear(struct gl_context *ctx,
>> +                            GLintptr offset, GLsizeiptr size,
>> +                            gl_format internalformat,
>> +                            unsigned int sizeOfFormat,
>> +                            GLenum format, GLenum type, const GLvoid*
>> data,
>> +                            struct gl_buffer_object *bufObj)
>> +{
>> +   GLenum internalformatBase;
>> +   GLubyte* src;
>> +
>> +   if (data == NULL) {
>> +      ctx->Driver.ClearBufferSubData(ctx, 0, bufObj->Size,
>> +                                     NULL, 0, bufObj);
>> +      return;
>> +   }
>> +
>> +   internalformatBase = _mesa_get_format_base_format(internalformat);
>> +
>> +   src = (GLubyte*) malloc(sizeOfFormat);
>> +   assert(src);
>> +   GLboolean success = _mesa_texstore(ctx, 1, internalformatBase,
>> +                                      internalformat, 0, &src, 1, 1, 1,
>> +                                      format, type, data, &ctx->Unpack);
>> +   assert(success);
>> +
>> +   ctx->Driver.ClearBufferSubData(ctx, offset, size,
>> +                                  src, sizeOfFormat, bufObj);
>> +   free(src);
>> +}
> 
> I think this function should just convert the user's clear value to the
> internal representation.  Then have the callers do the
> ctx->Driver.ClearBufferSubData() call.
> 
> And since sizeOfFormat will always be <=16 bytes, we could just use a
> GLubyte clearValue[16] buffer instead of malloc'ing the buffer.
> 
> 

I considered the second point but I thought that maybe there will be a
GL_RGAB32D (double) format soon and then it will stop working, as
opposed to this solution which is independent of new formats.

PS: sorry about all the little mistakes, I thought I found all in the
second version.