[Mesa-dev] [PATCH V2 07/10] mesa: Implement functions for clear_buffer_object extensions
Brian Paul
brianp at vmware.com
Thu Dec 12 07:16:21 PST 2013
On 12/12/2013 02:49 AM, Pi Tabred wrote:
>
>
> 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.
I should have suggested GLubyte[MAX_PIXEL_BYTES];
MAX_PIXEL_BYTES is defined in formats.h and will get bumped up when needed.
>
> PS: sorry about all the little mistakes, I thought I found all in the
> second version.
No problem. Thanks for your persistence!
-Brian
More information about the mesa-dev
mailing list