[Mesa-dev] [PATCH 06/21] main: Add entry point for NamedBufferData.
Ian Romanick
idr at freedesktop.org
Wed Jan 21 18:28:27 PST 2015
On 01/21/2015 05:40 PM, Laura Ekstrand wrote:
> ---
> src/mapi/glapi/gen/ARB_direct_state_access.xml | 9 ++-
> src/mesa/main/bufferobj.c | 77 ++++++++++++++++----------
> src/mesa/main/bufferobj.h | 13 ++++-
> src/mesa/main/tests/dispatch_sanity.cpp | 1 +
> 4 files changed, 69 insertions(+), 31 deletions(-)
>
> diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml b/src/mapi/glapi/gen/ARB_direct_state_access.xml
> index ff81c21..4b1ef6f 100644
> --- a/src/mapi/glapi/gen/ARB_direct_state_access.xml
> +++ b/src/mapi/glapi/gen/ARB_direct_state_access.xml
> @@ -21,7 +21,14 @@
> <param name="flags" type="GLbitfield" />
> </function>
>
> - <!-- Texture object functions -->
> + <function name="NamedBufferData" offset="assign">
> + <param name="buffer" type="GLuint" />
> + <param name="size" type="GLsizeiptr" />
> + <param name="data" type="const GLvoid *" />
> + <param name="usage" type="GLenum" />
> + </function>
> +
> + <!-- Texture object functions -->
I think you accidentally deleted a space before this comment.
>
> <function name="CreateTextures" offset="assign">
> <param name="target" type="GLenum" />
> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
> index 0977459..c77029f 100644
> --- a/src/mesa/main/bufferobj.c
> +++ b/src/mesa/main/bufferobj.c
> @@ -556,9 +556,9 @@ _mesa_total_buffer_object_memory(struct gl_context *ctx)
> * \sa glBufferDataARB, dd_function_table::BufferData.
> */
> static GLboolean
> -_mesa_buffer_data( struct gl_context *ctx, GLenum target, GLsizeiptrARB size,
> - const GLvoid * data, GLenum usage, GLenum storageFlags,
> - struct gl_buffer_object * bufObj )
> +_mesa_BufferData_sw(struct gl_context *ctx, GLenum target, GLsizeiptr size,
> + const GLvoid *data, GLenum usage, GLenum storageFlags,
> + struct gl_buffer_object *bufObj)
We should pick a different name for this. Names with this pattern are
(almost exclusively) direct API entry points. Maybe _swrast_buffer_data?
Alternately, if the "new" _mesa_buffer_data will only ever be called
from the same file, we could just call that buffer_data and make it static.
> {
> void * new_data;
>
> @@ -1112,7 +1112,7 @@ _mesa_init_buffer_object_functions(struct dd_function_table *driver)
> /* GL_ARB_vertex/pixel_buffer_object */
> driver->NewBufferObject = _mesa_new_buffer_object;
> driver->DeleteBuffer = _mesa_delete_buffer_object;
> - driver->BufferData = _mesa_buffer_data;
> + driver->BufferData = _mesa_BufferData_sw;
> driver->BufferSubData = _mesa_buffer_subdata;
> driver->GetBufferSubData = _mesa_buffer_get_subdata;
> driver->UnmapBuffer = _mesa_buffer_unmap;
> @@ -1474,23 +1474,22 @@ _mesa_NamedBufferStorage(GLuint buffer, GLsizeiptr size, const GLvoid *data,
> }
>
>
> -
> -void GLAPIENTRY
> -_mesa_BufferData(GLenum target, GLsizeiptrARB size,
> - const GLvoid * data, GLenum usage)
> +void
> +_mesa_buffer_data(struct gl_context *ctx, struct gl_buffer_object *bufObj,
> + GLenum target, GLsizeiptr size, const GLvoid *data,
> + GLenum usage, const char *func)
> {
> - GET_CURRENT_CONTEXT(ctx);
> - struct gl_buffer_object *bufObj;
> bool valid_usage;
>
> if (MESA_VERBOSE & VERBOSE_API)
> - _mesa_debug(ctx, "glBufferData(%s, %ld, %p, %s)\n",
> + _mesa_debug(ctx, "%s(%s, %ld, %p, %s)\n",
> + func,
> _mesa_lookup_enum_by_nr(target),
> (long int) size, data,
> _mesa_lookup_enum_by_nr(usage));
>
> if (size < 0) {
> - _mesa_error(ctx, GL_INVALID_VALUE, "glBufferDataARB(size < 0)");
> + _mesa_error(ctx, GL_INVALID_VALUE, "%s(size < 0)", func);
> return;
> }
>
> @@ -1519,16 +1518,13 @@ _mesa_BufferData(GLenum target, GLsizeiptrARB size,
> }
>
> if (!valid_usage) {
> - _mesa_error(ctx, GL_INVALID_ENUM, "glBufferData(usage)");
> + _mesa_error(ctx, GL_INVALID_ENUM, "%s(invalid usage: %s)", func,
> + _mesa_lookup_enum_by_nr(usage));
> return;
> }
>
> - bufObj = get_buffer(ctx, "glBufferDataARB", target, GL_INVALID_OPERATION);
> - if (!bufObj)
> - return;
> -
> if (bufObj->Immutable) {
> - _mesa_error(ctx, GL_INVALID_OPERATION, "glBufferData(immutable)");
> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(immutable)", func);
> return;
> }
>
> @@ -1539,25 +1535,50 @@ _mesa_BufferData(GLenum target, GLsizeiptrARB size,
>
> bufObj->Written = GL_TRUE;
>
> -#ifdef VBO_DEBUG
> - printf("glBufferDataARB(%u, sz %ld, from %p, usage 0x%x)\n",
> - bufObj->Name, size, data, usage);
> -#endif
> -
> -#ifdef BOUNDS_CHECK
> - size += 100;
> -#endif
> -
This change by itself is not okay. If someone builds with BOUNDS_CHECK
defined, the code in _mesa_MapBuffer and _mesa_UnmapBuffer will explode.
I don't know how much utility there is to BOUNDS_CHECK or VBO_DEBUG.
I'd be inclined to send separate patches that completely remove each of
those.
It looks like BOUNDS_CHECK was added by Brian Paul in commit 4a7fd632.
Maybe he has an opinion...
> ASSERT(ctx->Driver.BufferData);
> if (!ctx->Driver.BufferData(ctx, target, size, data, usage,
> GL_MAP_READ_BIT |
> GL_MAP_WRITE_BIT |
> GL_DYNAMIC_STORAGE_BIT,
> bufObj)) {
> - _mesa_error(ctx, GL_OUT_OF_MEMORY, "glBufferDataARB()");
> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "%s", func);
> }
> }
>
> +void GLAPIENTRY
> +_mesa_BufferData(GLenum target, GLsizeiptr size,
> + const GLvoid *data, GLenum usage)
> +{
> + GET_CURRENT_CONTEXT(ctx);
> + struct gl_buffer_object *bufObj;
> +
> + bufObj = get_buffer(ctx, "glBufferData", target, GL_INVALID_OPERATION);
> + if (!bufObj)
> + return;
> +
> + _mesa_buffer_data(ctx, bufObj, target, size, data, usage,
> + "glBufferData");
> +}
> +
> +void GLAPIENTRY
> +_mesa_NamedBufferData(GLuint buffer, GLsizeiptr size, const GLvoid *data,
> + GLenum usage)
> +{
> + GET_CURRENT_CONTEXT(ctx);
> + struct gl_buffer_object *bufObj;
> +
> + bufObj = _mesa_lookup_bufferobj_err(ctx, buffer, "glNamedBufferData");
> + if (!bufObj)
> + return;
> +
> + /*
> + * In direct state access, buffer objects have an unspecified target since
> + * they are not required to be bound.
> + */
Tiny nit... the opening /* should go on the same like with the start of
the comment text. It's probably not worth going through the series to
change the existing occurrences, but new code should use that style.
> + _mesa_buffer_data(ctx, bufObj, GL_NONE, size, data, usage,
> + "glNamedBufferData");
> +}
> +
>
> void GLAPIENTRY
> _mesa_BufferSubData(GLenum target, GLintptrARB offset,
> diff --git a/src/mesa/main/bufferobj.h b/src/mesa/main/bufferobj.h
> index 0537bd4..9801c87 100644
> --- a/src/mesa/main/bufferobj.h
> +++ b/src/mesa/main/bufferobj.h
> @@ -135,6 +135,11 @@ _mesa_buffer_storage(struct gl_context *ctx, struct gl_buffer_object *bufObj,
> GLbitfield flags, const char *func);
>
> extern void
> +_mesa_buffer_data(struct gl_context *ctx, struct gl_buffer_object *bufObj,
> + GLenum target, GLsizeiptr size, const GLvoid *data,
> + GLenum usage, const char *func);
> +
> +extern void
> _mesa_buffer_unmap_all_mappings(struct gl_context *ctx,
> struct gl_buffer_object *bufObj);
>
> @@ -172,8 +177,12 @@ _mesa_NamedBufferStorage(GLuint buffer, GLsizeiptr size, const GLvoid *data,
> GLbitfield flags);
>
> void GLAPIENTRY
> -_mesa_BufferData(GLenum target, GLsizeiptrARB size,
> - const GLvoid * data, GLenum usage);
> +_mesa_BufferData(GLenum target, GLsizeiptr size,
> + const GLvoid *data, GLenum usage);
> +
> +void GLAPIENTRY
> +_mesa_NamedBufferData(GLuint buffer, GLsizeiptr size,
> + const GLvoid *data, GLenum usage);
>
> void GLAPIENTRY
> _mesa_BufferSubData(GLenum target, GLintptrARB offset,
> diff --git a/src/mesa/main/tests/dispatch_sanity.cpp b/src/mesa/main/tests/dispatch_sanity.cpp
> index 131d15e..3c87b5e 100644
> --- a/src/mesa/main/tests/dispatch_sanity.cpp
> +++ b/src/mesa/main/tests/dispatch_sanity.cpp
> @@ -957,6 +957,7 @@ const struct function gl_core_functions_possible[] = {
> /* GL_ARB_direct_state_access */
> { "glCreateBuffers", 45, -1 },
> { "glNamedBufferStorage", 45, -1 },
> + { "glNamedBufferData", 45, -1 },
> { "glCreateTextures", 45, -1 },
> { "glTextureStorage1D", 45, -1 },
> { "glTextureStorage2D", 45, -1 },
>
More information about the mesa-dev
mailing list