[Mesa-dev] [PATCH V2 07/10] mesa: Implement functions for clear_buffer_object extensions
Brian Paul
brianp at vmware.com
Fri Dec 13 16:09:16 PST 2013
On 12/13/2013 03:17 PM, Pi Tabred wrote:
> I tried to tackle this issue:
>>> There is still the issue regarding the following:
>>> According to the spec, it should be possible to clear some part of a
>>> buffer, even if a different, non-overlapping part is mapped, this is
>>> currently not possible. It was suggested to implement ClearBufferSubData
>>> using MapBufferRange. However, this does not work if a part of the
>>> buffer is already mapped.
>>> I am not sure how to handle this. Maybe that's something that has to be
>>> done inside the different drivers?
>
> I used BufferSubData to transfer the data, however according to the spec
> BufferSubData is only guaranteed to work if the complete buffer is
> unmapped.
OK, that's probably as good as we can do for now.
> - _mesa_buffer_clear_subdata: default callback for dd function table
> - _mesa_ClearBufferData: API function
> - _mesa_ClearBufferSubData: API function
> - validate_clear_buffer_format: helper function, check if the
> internalformat,
> format and type parameter are legal
> - convert_clear_buffer_data: helper function, convert the supplied data
> to the desired internalformat.
I don't think you need to list the functions in the commit message, but
not a big deal.
> ---
> src/mesa/main/bufferobj.c | 259
> ++++++++++++++++++++++++++++++++++++++++++++++
> src/mesa/main/bufferobj.h | 4 +
> 2 files changed, 263 insertions(+)
>
> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
> index 1219891..fb09aa9 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,96 @@ buffer_object_subdata_range_good(struct gl_context
> * ctx, GLenum target,
>
>
> /**
> + * Test the format and type parameters and set the GL error code for
> + * \c glClearBufferData and \c glClearBufferSubData.
> + *
> + * \param ctx GL context.
> + * \param internalformat Format to which the data is to be converted.
> + * \param format Format of the supplied data.
> + * \param type Type of the supplied data.
> + * \param caller Name of calling function for recording errors.
> + * \return If internalformat, format and type are legal the gl_format
> + * corresponding to internalformat, otherwise MESA_FORMAT_NONE.
> + *
> + * \sa glClearBufferData and glClearBufferSubData
> + */
> +static gl_format
> +validate_clear_buffer_format(struct gl_context *ctx,
> + GLenum internalformat,
> + GLenum format, GLenum type,
> + const char* caller)
"const char *caller"
> +{
> + gl_format mesaFormat;
> + GLenum errorFormatType;
> +
> + mesaFormat = _mesa_validate_texbuffer_format(ctx, internalformat);
> + if (mesaFormat == 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(mesaFormat)) {
> + _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 mesaFormat;
> +}
> +
> +
> +/**
> + * Convert user-specified clear value to the specified internal format.
> + *
> + * \param ctx GL context.
> + * \param internalformat Format to which the data is converted.
> + * \param clearValue Pointer, store converted data here.
> + * \param format Format of the supplied data.
> + * \param type Type of the supplied data.
> + * \param data Data which is to be converted to internalformat
> + *
> + * \sa glClearBufferData, glClearBufferSubData
> + */
> +static void
> +convert_clear_buffer_data(struct gl_context *ctx,
> + gl_format internalformat,
> + GLubyte *clearValue, GLenum format, GLenum type,
> + const GLvoid *data)
> +{
> + GLenum internalformatBase;
> +
> + internalformatBase = _mesa_get_format_base_format(internalformat);
> +
> +
> + GLboolean success = _mesa_texstore(ctx, 1, internalformatBase,
This variable needs to be declared at the top of the function.
> + internalformat, 0, &clearValue,
> + 1, 1, 1,
> + format, type, data, &ctx->Unpack);
> + assert(success);
_mesa_texstore() should only return false if we run out of memory. So,
we should probably call _mesa_error(GL_OUT_OF_MEMORY) here if it fails.
I'll put a comment on _mesa_texstore().
> +}
> +
> +
> +/**
> * Allocate and initialize a new buffer object.
> *
> * Default callback for the \c dd_function_table::NewBufferObject() hook.
> @@ -547,6 +640,72 @@ _mesa_buffer_get_subdata( struct gl_context *ctx,
> GLintptrARB offset,
>
>
> /**
> + * Clear a subrange of buffer object with supplied data. If the data range
> + * specified by \c size + \c offset extends beyond the end of the buffer
> + * the error INVALID_VALUE is generated, if data is NULL the buffer is
> filled
> + * with zeros.
> + *
> + * This is the default callback for \c
> dd_function_table::ClearBufferSubData()
> + * Note that all GL error checking will have been done already.
> + *
> + * \param ctx GL context.
> + * \param offset Offset of the first byte to be cleared.
> + * \param size Size, in bytes, of the to be cleared range.
> + * \param clearValue Source of the data.
> + * \param clearValueSize Size, in bytes, of the supplied data.
> + * \param bufObj Object to be cleared.
> + *
> + * \sa glClearBufferSubDataARB, dd_function_table::ClearBufferSubData.
> + */
> +static void
> +_mesa_buffer_clear_subdata(struct gl_context *ctx,
> + GLintptr offset, GLsizeiptr size,
> + const GLvoid *clearValue,
> + GLsizeiptr clearValueSize,
> + struct gl_buffer_object *bufObj)
> +{
> + GLsizeiptr i;
> + GLubyte *dest;
> +
> + if (_mesa_bufferobj_mapped(bufObj)) {
> + GLubyte *data = malloc(size);
> + GLubyte *dataStart = data;
> +
Please add a data==NULL check (GL_OUT_OF_MEMORY) here.
> + if (clearValue == NULL) {
> + memset(data, 0, size);
> + }
> + else {
> + for (i = 0; i < size/clearValueSize; ++i) {
> + memcpy(data, clearValue, clearValueSize);
> + data += clearValueSize;
> + }
> + }
> + ctx->Driver.BufferSubData(ctx, offset, size, dataStart, bufObj);
> + return;
> + }
> +
> + ASSERT(ctx->Driver.MapBufferRange);
> + dest = ctx->Driver.MapBufferRange(ctx, offset, size,
> + GL_MAP_WRITE_BIT |
> + GL_MAP_INVALIDATE_RANGE_BIT,
> + bufObj);
> +
> + /* Clear range to zero if clearValue == NULL */
> + if (clearValue == NULL) {
> + memset(dest, 0, size);
> + ctx->Driver.UnmapBuffer(ctx, bufObj);
> + return;
> + }
> +
> + for (i = 0; i < size/clearValueSize; ++i) {
> + memcpy(dest, clearValue, clearValueSize);
> + dest += clearValueSize;
> + }
> + ctx->Driver.UnmapBuffer(ctx, bufObj);
> +}
> +
> +
> +/**
> * Default fallback for \c dd_function_table::MapBufferRange().
> * Called via glMapBufferRange().
> */
> @@ -852,6 +1011,9 @@ _mesa_init_buffer_object_functions(struct
> dd_function_table *driver)
> driver->GetBufferSubData = _mesa_buffer_get_subdata;
> driver->UnmapBuffer = _mesa_buffer_unmap;
>
> + /* GL_ARB_clear_buffer_object */
> + driver->ClearBufferSubData = _mesa_buffer_clear_subdata;
> +
> /* GL_ARB_map_buffer_range */
> driver->MapBufferRange = _mesa_buffer_map_range;
> driver->FlushMappedBufferRange = _mesa_buffer_flush_mapped_range;
> @@ -1182,6 +1344,103 @@ _mesa_GetBufferSubData(GLenum target,
> GLintptrARB offset,
> }
>
>
> +void GLAPIENTRY
> +_mesa_ClearBufferData(GLenum target, GLenum internalformat, GLenum format,
> + GLenum type, const GLvoid* data)
> +{
> + GET_CURRENT_CONTEXT(ctx);
> + struct gl_buffer_object* bufObj;
> + gl_format mesaFormat;
> + GLubyte clearValue[MAX_PIXEL_BYTES];
> + GLsizeiptr clearValueSize;
> +
> + bufObj = get_buffer(ctx, "glClearBufferData", target, GL_INVALID_VALUE);
> + if (!bufObj) {
> + return;
> + }
> +
> + if (_mesa_bufferobj_mapped(bufObj)) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "glClearBufferData(buffer currently mapped)");
> + return;
> + }
> +
> + mesaFormat = validate_clear_buffer_format(ctx, internalformat,
> + format, type,
> + "glClearBufferData");
> + if (mesaFormat == MESA_FORMAT_NONE) {
> + return;
> + }
> +
> +
> + clearValueSize = _mesa_get_format_bytes(mesaFormat);
> + if (bufObj->Size % clearValueSize != 0) {
> + _mesa_error(ctx, GL_INVALID_VALUE,
> + "glClearBufferData(size is not a multiple of
> internalformat size)");
> + return;
> + }
> +
> + if (data == NULL) {
> + ctx->Driver.ClearBufferSubData(ctx, 0, bufObj->Size,
> + NULL, 0, bufObj);
> + return;
> + }
> +
> + convert_clear_buffer_data(ctx, mesaFormat, clearValue,
> + format, type, data);
> +
> + ctx->Driver.ClearBufferSubData(ctx, 0, bufObj->Size,
> + clearValue, clearValueSize, bufObj);
> +}
> +
> +
> +void GLAPIENTRY
> +_mesa_ClearBufferSubData(GLenum target, GLenum internalformat,
> + GLintptr offset, GLsizeiptr size,
> + GLenum format, GLenum type,
> + const GLvoid* data)
> +{
> + GET_CURRENT_CONTEXT(ctx);
> + struct gl_buffer_object* bufObj;
> + gl_format mesaFormat;
> + GLubyte clearValue[MAX_PIXEL_BYTES];
> + GLsizeiptr clearValueSize;
> +
> + bufObj = buffer_object_subdata_range_good(ctx, target, offset, size,
> + true, GL_INVALID_VALUE,
> + "glClearBufferSubData");
> + if (!bufObj) {
> + return;
> + }
> +
> + mesaFormat = validate_clear_buffer_format(ctx, internalformat,
> + format, type,
> + "glClearBufferData");
> + 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) {
> + ctx->Driver.ClearBufferSubData(ctx, offset, size,
> + NULL, 0, bufObj);
> + return;
> + }
> +
> + convert_clear_buffer_data(ctx, mesaFormat, clearValue,
> + format, type, data);
> +
> + ctx->Driver.ClearBufferSubData(ctx, offset, size,
> + clearValue, clearValueSize, bufObj);
> +}
> +
> +
> void * GLAPIENTRY
> _mesa_MapBuffer(GLenum target, GLenum access)
> {
> diff --git a/src/mesa/main/bufferobj.h b/src/mesa/main/bufferobj.h
> index 0b898a2..b38519f 100644
> --- a/src/mesa/main/bufferobj.h
> +++ b/src/mesa/main/bufferobj.h
> @@ -120,6 +120,10 @@ void GLAPIENTRY
> _mesa_BufferSubData(GLenum target, GLintptrARB offset, GLsizeiptrARB
> size, const GLvoid * data);
> void GLAPIENTRY
> _mesa_GetBufferSubData(GLenum target, GLintptrARB offset, GLsizeiptrARB
> size, void * data);
> +void GLAPIENTRY
> +_mesa_ClearBufferData(GLenum target, GLenum internalformat, GLenum
> format, GLenum type, const GLvoid * data);
> +void GLAPIENTRY
> +_mesa_ClearBufferSubData(GLenum target, GLenum internalformat, GLintptr
> offset, GLsizeiptr size, GLenum format, GLenum type, const GLvoid * data);
> void * GLAPIENTRY
> _mesa_MapBuffer(GLenum target, GLenum access);
> GLboolean GLAPIENTRY
>
The rest looks good.
-Brian
More information about the mesa-dev
mailing list