<div dir="ltr">Again, I think this would be more readable if the refactor was in its own commit with a little more explanation what's going on. Then add DSA. </div><div class="gmail_extra"> <div class="gmail_quote">On Wed, Jan 21, 2015 at 5:40 PM, Laura Ekstrand <<a href="mailto:laura@jlekstrand.net" target="_blank">laura@jlekstrand.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">--- src/mapi/glapi/gen/ARB_direct_state_access.xml | 14 ++ src/mesa/main/bufferobj.c | 327 ++++++++++++------------- src/mesa/main/bufferobj.h | 20 +- src/mesa/main/tests/dispatch_sanity.cpp | 2 + 4 files changed, 189 insertions(+), 174 deletions(-) diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml b/src/mapi/glapi/gen/ARB_direct_state_access.xml index 4b29e00..7727f0d 100644 --- a/src/mapi/glapi/gen/ARB_direct_state_access.xml +++ b/src/mapi/glapi/gen/ARB_direct_state_access.xml @@ -61,6 +61,20 @@ <param name="data" type="const GLvoid *" /> </function> + <function name="MapNamedBuffer" offset="assign"> + <return type="GLvoid *" /> + <param name="buffer" type="GLuint" /> + <param name="access" type="GLenum" /> + </function> + + <function name="MapNamedBufferRange" offset="assign"> + <return type="GLvoid *" /> + <param name="buffer" type="GLuint" /> + <param name="offset" type="GLintptr" /> + <param name="length" type="GLsizeiptr" /> + <param name="access" type="GLbitfield" /> + </function> +  <function name="CreateTextures" offset="assign"> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c index 1ec1681..16c0faa 100644 --- a/src/mesa/main/bufferobj.c +++ b/src/mesa/main/bufferobj.c @@ -701,7 +701,7 @@ _mesa_ClearBufferSubData_sw(struct gl_context *ctx, * Called via glMapBufferRange(). */ static void * -_mesa_buffer_map_range( struct gl_context *ctx, GLintptr offset, +_mesa_MapBufferRange_sw(struct gl_context *ctx, GLintptr offset, GLsizeiptr length, GLbitfield access, struct gl_buffer_object *bufObj, gl_map_buffer_index index) @@ -1116,7 +1116,7 @@ _mesa_init_buffer_object_functions(struct dd_function_table *driver) driver->ClearBufferSubData = _mesa_ClearBufferSubData_sw; /* GL_ARB_map_buffer_range */ - driver->MapBufferRange = _mesa_buffer_map_range; + driver->MapBufferRange = _mesa_MapBufferRange_sw; driver->FlushMappedBufferRange = _mesa_buffer_flush_mapped_range; /* GL_ARB_copy_buffer */ @@ -1794,116 +1794,6 @@ _mesa_ClearNamedBufferSubData(GLuint buffer, GLenum internalformat, } -void * GLAPIENTRY -_mesa_MapBuffer(GLenum target, GLenum access) -{ - GET_CURRENT_CONTEXT(ctx); - struct gl_buffer_object * bufObj; - GLbitfield accessFlags; - void *map; - bool valid_access; - - ASSERT_OUTSIDE_BEGIN_END_WITH_RETVAL(ctx, NULL); - - switch (access) { - case GL_READ_ONLY_ARB: - accessFlags = GL_MAP_READ_BIT; - valid_access = _mesa_is_desktop_gl(ctx); - break; - case GL_WRITE_ONLY_ARB: - accessFlags = GL_MAP_WRITE_BIT; - valid_access = true; - break; - case GL_READ_WRITE_ARB: - accessFlags = GL_MAP_READ_BIT | GL_MAP_WRITE_BIT; - valid_access = _mesa_is_desktop_gl(ctx); - break; - default: - valid_access = false; - break; - } - - if (!valid_access) { - _mesa_error(ctx, GL_INVALID_ENUM, "glMapBufferARB(access)"); - return NULL; - } - - bufObj = get_buffer(ctx, "glMapBufferARB", target, GL_INVALID_OPERATION); - if (!bufObj) - return NULL; - - if (accessFlags & GL_MAP_READ_BIT && - !(bufObj->StorageFlags & GL_MAP_READ_BIT)) { - _mesa_error(ctx, GL_INVALID_OPERATION, - "glMapBuffer(invalid read flag)"); - return NULL; - } - - if (accessFlags & GL_MAP_WRITE_BIT && - !(bufObj->StorageFlags & GL_MAP_WRITE_BIT)) { - _mesa_error(ctx, GL_INVALID_OPERATION, - "glMapBuffer(invalid write flag)"); - return NULL; - } - - if (_mesa_bufferobj_mapped(bufObj, MAP_USER)) { - _mesa_error(ctx, GL_INVALID_OPERATION, "glMapBufferARB(already mapped)"); - return NULL; - } - - if (!bufObj->Size) { - _mesa_error(ctx, GL_OUT_OF_MEMORY, - "glMapBuffer(buffer size = 0)"); - return NULL; - } - - ASSERT(ctx->Driver.MapBufferRange); - map = ctx->Driver.MapBufferRange(ctx, 0, bufObj->Size, accessFlags, bufObj, - MAP_USER); - if (!map) { - _mesa_error(ctx, GL_OUT_OF_MEMORY, "glMapBufferARB(map failed)"); - return NULL; - } - else { - /* The driver callback should have set these fields. - * This is important because other modules (like VBO) might call - * the driver function directly. - */ - ASSERT(bufObj->Mappings[MAP_USER].Pointer == map); - ASSERT(bufObj->Mappings[MAP_USER].Length == bufObj->Size); - ASSERT(bufObj->Mappings[MAP_USER].Offset == 0); - bufObj->Mappings[MAP_USER].AccessFlags = accessFlags; - } - - if (access == GL_WRITE_ONLY_ARB || access == GL_READ_WRITE_ARB) - bufObj->Written = GL_TRUE; - -#ifdef VBO_DEBUG - printf("glMapBufferARB(%u, sz %ld, access 0x%x)\n", - bufObj->Name, bufObj->Size, access); - if (access == GL_WRITE_ONLY_ARB) { - GLuint i; - GLubyte *b = (GLubyte *) bufObj->Pointer; - for (i = 0; i < bufObj->Size; i++) - b[i] = i & 0xff; - } -#endif - -#ifdef BOUNDS_CHECK - { - GLubyte *buf = (GLubyte *) bufObj->Pointer; - GLuint i; - /* buffer is 100 bytes larger than requested, fill with magic value */ - for (i = 0; i < 100; i++) { - buf[bufObj->Size - i - 1] = 123; - } - } -#endif - - return bufObj->Mappings[MAP_USER].Pointer; -} - - GLboolean GLAPIENTRY _mesa_UnmapBuffer(GLenum target) { @@ -2227,35 +2117,33 @@ _mesa_CopyNamedBufferSubData(GLuint readBuffer, GLuint writeBuffer, } -/** - * See GL_ARB_map_buffer_range spec - */ -void * GLAPIENTRY -_mesa_MapBufferRange(GLenum target, GLintptr offset, GLsizeiptr length, - GLbitfield access) +void * +_mesa_map_buffer_range(struct gl_context *ctx, + struct gl_buffer_object *bufObj, + GLintptr offset, GLsizeiptr length, + GLbitfield access, const char *func) { - GET_CURRENT_CONTEXT(ctx); - struct gl_buffer_object *bufObj; void *map; GLbitfield allowed_access; ASSERT_OUTSIDE_BEGIN_END_WITH_RETVAL(ctx, NULL); - if (!ctx->Extensions.ARB_map_buffer_range) { - _mesa_error(ctx, GL_INVALID_OPERATION, - "glMapBufferRange(extension not supported)"); + if (offset < 0) { + _mesa_error(ctx, GL_INVALID_VALUE, + "%s(offset %ld < 0)", func, (long) offset); return NULL; } - if (offset < 0) { + if (length < 0) { _mesa_error(ctx, GL_INVALID_VALUE, - "glMapBufferRange(offset = %ld)", (long)offset); + "%s(length %ld < 0)", func, (long) length); return NULL; } - if (length < 0) { + if (offset + length > bufObj->Size) { _mesa_error(ctx, GL_INVALID_VALUE, - "glMapBufferRange(length = %ld)", (long)length); + "%s(offset %d + length %d > bufObj->Size %d)", func, + (int) offset, (int) length, (int) bufObj->Size); return NULL; } @@ -2265,13 +2153,30 @@ _mesa_MapBufferRange(GLenum target, GLintptr offset, GLsizeiptr length, * conditions: * * * <length> is zero." + * + * Additionally, page 94 of the PDF of the OpenGL 4.5 core spec + * (30.10.2014) also says this, so it's no longer allowed for desktop GL, + * either. */ - if (_mesa_is_gles(ctx) && length == 0) { + if (length == 0) { + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(length = 0)", func); + return NULL; + } + + if (_mesa_bufferobj_mapped(bufObj, MAP_USER)) { _mesa_error(ctx, GL_INVALID_OPERATION, - "glMapBufferRange(length = 0)"); + "%s(buffer already mapped)", func); + return NULL; + } + + if (!bufObj->Size) { + _mesa_error(ctx, GL_OUT_OF_MEMORY, "%s(buffer size = 0)", func); return NULL; } + + /* Check access bits */ + allowed_access = GL_MAP_READ_BIT | GL_MAP_WRITE_BIT | GL_MAP_INVALIDATE_RANGE_BIT | @@ -2285,14 +2190,15 @@ _mesa_MapBufferRange(GLenum target, GLintptr offset, GLsizeiptr length, } if (access & ~allowed_access) { - /* generate an error if any other than allowed bit is set */ - _mesa_error(ctx, GL_INVALID_VALUE, "glMapBufferRange(access)"); + /* generate an error if any bits other than those allowed are set */ + _mesa_error(ctx, GL_INVALID_VALUE, + "%s(access has undefined bits set)", func); return NULL; } if ((access & (GL_MAP_READ_BIT | GL_MAP_WRITE_BIT)) == 0) { _mesa_error(ctx, GL_INVALID_OPERATION, - "glMapBufferRange(access indicates neither read or write)"); + "%s(access indicates neither read or write)", func); return NULL; } @@ -2301,82 +2207,53 @@ _mesa_MapBufferRange(GLenum target, GLintptr offset, GLsizeiptr length, GL_MAP_INVALIDATE_BUFFER_BIT | GL_MAP_UNSYNCHRONIZED_BIT))) { _mesa_error(ctx, GL_INVALID_OPERATION, - "glMapBufferRange(invalid access flags)"); + "%s(read access with disallowed bits)", func); return NULL; } if ((access & GL_MAP_FLUSH_EXPLICIT_BIT) && ((access & GL_MAP_WRITE_BIT) == 0)) { _mesa_error(ctx, GL_INVALID_OPERATION, - "glMapBufferRange(invalid access flags)"); + "%s(access has flush explicit without write)", func); return NULL; } - bufObj = get_buffer(ctx, "glMapBufferRange", target, GL_INVALID_OPERATION); - if (!bufObj) - return NULL; - if (access & GL_MAP_READ_BIT && !(bufObj->StorageFlags & GL_MAP_READ_BIT)) { _mesa_error(ctx, GL_INVALID_OPERATION, - "glMapBufferRange(invalid read flag)"); + "%s(buffer does not allow read access)", func); return NULL; } if (access & GL_MAP_WRITE_BIT && !(bufObj->StorageFlags & GL_MAP_WRITE_BIT)) { _mesa_error(ctx, GL_INVALID_OPERATION, - "glMapBufferRange(invalid write flag)"); - return NULL; - } - - if (access & GL_MAP_COHERENT_BIT && - !(bufObj->StorageFlags & GL_MAP_COHERENT_BIT)) { - _mesa_error(ctx, GL_INVALID_OPERATION, - "glMapBufferRange(invalid coherent flag)"); + "%s(buffer does not allow write access)", func); return NULL; } if (access & GL_MAP_PERSISTENT_BIT && !(bufObj->StorageFlags & GL_MAP_PERSISTENT_BIT)) { _mesa_error(ctx, GL_INVALID_OPERATION, - "glMapBufferRange(invalid persistent flag)"); - return NULL; - } - - if (offset + length > bufObj->Size) { - _mesa_error(ctx, GL_INVALID_VALUE, - "glMapBufferRange(offset + length > size)"); + "%s(buffer does not allow persistent access)", func); return NULL; } - if (_mesa_bufferobj_mapped(bufObj, MAP_USER)) { + if (access & GL_MAP_COHERENT_BIT && + !(bufObj->StorageFlags & GL_MAP_COHERENT_BIT)) { _mesa_error(ctx, GL_INVALID_OPERATION, - "glMapBufferRange(buffer already mapped)"); + "%s(buffer does not allow coherent access)", func); return NULL; } - if (!bufObj->Size) { - _mesa_error(ctx, GL_OUT_OF_MEMORY, - "glMapBufferRange(buffer size = 0)"); - return NULL; - } - /* Mapping zero bytes should return a non-null pointer. */ - if (!length) { - static long dummy = 0; - bufObj->Mappings[MAP_USER].Pointer = &dummy; - bufObj->Mappings[MAP_USER].Length = length; - bufObj->Mappings[MAP_USER].Offset = offset; - bufObj->Mappings[MAP_USER].AccessFlags = access; - return bufObj->Mappings[MAP_USER].Pointer; - } + /* Perform mapping */ ASSERT(ctx->Driver.MapBufferRange); map = ctx->Driver.MapBufferRange(ctx, offset, length, access, bufObj, MAP_USER); if (!map) { - _mesa_error(ctx, GL_OUT_OF_MEMORY, "glMapBufferARB(map failed)"); + _mesa_error(ctx, GL_OUT_OF_MEMORY, "%s(map failed)", func); } else { /* The driver callback should have set all these fields. @@ -2392,6 +2269,114 @@ _mesa_MapBufferRange(GLenum target, GLintptr offset, GLsizeiptr length, return map; } +void * GLAPIENTRY +_mesa_MapBufferRange(GLenum target, GLintptr offset, GLsizeiptr length, + GLbitfield access) +{ + GET_CURRENT_CONTEXT(ctx); + struct gl_buffer_object *bufObj; + + bufObj = get_buffer(ctx, "glMapBufferRange", target, GL_INVALID_OPERATION); + if (!bufObj) + return NULL; + + if (!ctx->Extensions.ARB_map_buffer_range) { + _mesa_error(ctx, GL_INVALID_OPERATION, + "glMapBufferRange(ARB_map_buffer_range not supported)"); + return NULL; + } + + return _mesa_map_buffer_range(ctx, bufObj, offset, length, access, + "glMapBufferRange"); +} + +void * GLAPIENTRY +_mesa_MapNamedBufferRange(GLuint buffer, GLintptr offset, GLsizeiptr length, + GLbitfield access) +{ + GET_CURRENT_CONTEXT(ctx); + struct gl_buffer_object *bufObj; + + bufObj = _mesa_lookup_bufferobj_err(ctx, buffer, "glMapNamedBufferRange"); + if (!bufObj) + return NULL; + + if (!ctx->Extensions.ARB_map_buffer_range) { + _mesa_error(ctx, GL_INVALID_OPERATION, + "glMapNamedBufferRange(" + "ARB_map_buffer_range not supported)"); + return NULL; + } + + return _mesa_map_buffer_range(ctx, bufObj, offset, length, access, + "glMapNamedBufferRange"); +} + +/** + * Converts GLenum access from MapBuffer and MapNamedBuffer into + * flags for input to _mesa_map_buffer_range. + * + * \return true if the type of requested access is permissible. + */ +static bool +get_map_buffer_access_flags(struct gl_context *ctx, GLenum access, + GLbitfield *flags) +{ + switch (access) { + case GL_READ_ONLY_ARB: + *flags = GL_MAP_READ_BIT; + return _mesa_is_desktop_gl(ctx); + case GL_WRITE_ONLY_ARB: + *flags = GL_MAP_WRITE_BIT; + return true; + case GL_READ_WRITE_ARB: + *flags = GL_MAP_READ_BIT | GL_MAP_WRITE_BIT; + return _mesa_is_desktop_gl(ctx); + default: + return false; + } +} + +void * GLAPIENTRY +_mesa_MapBuffer(GLenum target, GLenum access) +{ + GET_CURRENT_CONTEXT(ctx); + struct gl_buffer_object *bufObj; + GLbitfield accessFlags; + + if (!get_map_buffer_access_flags(ctx, access, &accessFlags)) { + _mesa_error(ctx, GL_INVALID_ENUM, "glMapBuffer(invalid access)"); + return NULL; + } + + bufObj = get_buffer(ctx, "glMapBuffer", target, GL_INVALID_OPERATION); + if (!bufObj) + return NULL; + + return _mesa_map_buffer_range(ctx, bufObj, 0, bufObj->Size, accessFlags, + "glMapBuffer"); +} + +void * GLAPIENTRY +_mesa_MapNamedBuffer(GLuint buffer, GLenum access) +{ + GET_CURRENT_CONTEXT(ctx); + struct gl_buffer_object *bufObj; + GLbitfield accessFlags; + + if (!get_map_buffer_access_flags(ctx, access, &accessFlags)) { + _mesa_error(ctx, GL_INVALID_ENUM, "glMapNamedBuffer(invalid access)"); + return NULL; + } + + bufObj = _mesa_lookup_bufferobj_err(ctx, buffer, "glMapNamedBuffer"); + if (!bufObj) + return NULL; + + return _mesa_map_buffer_range(ctx, bufObj, 0, bufObj->Size, accessFlags, + "glMapNamedBuffer"); +} + /** * See GL_ARB_map_buffer_range spec diff --git a/src/mesa/main/bufferobj.h b/src/mesa/main/bufferobj.h index b6e833d..b34162e 100644 --- a/src/mesa/main/bufferobj.h +++ b/src/mesa/main/bufferobj.h @@ -155,6 +155,12 @@ _mesa_copy_buffer_sub_data(struct gl_context *ctx, GLintptr readOffset, GLintptr writeOffset, GLsizeiptr size, const char *func); +extern void * +_mesa_map_buffer_range(struct gl_context *ctx, + struct gl_buffer_object *bufObj, + GLintptr offset, GLsizeiptr length, + GLbitfield access, const char *func); + extern void _mesa_ClearBufferSubData_sw(struct gl_context *ctx, GLintptr offset, GLsizeiptr size, @@ -239,9 +245,6 @@ _mesa_ClearNamedBufferSubData(GLuint buffer, GLenum internalformat, GLenum format, GLenum type, const GLvoid *data); -void * GLAPIENTRY -_mesa_MapBuffer(GLenum target, GLenum access); - GLboolean GLAPIENTRY _mesa_UnmapBuffer(GLenum target); @@ -268,6 +271,17 @@ void * GLAPIENTRY _mesa_MapBufferRange(GLenum target, GLintptr offset, GLsizeiptr length, GLbitfield access); +void * GLAPIENTRY +_mesa_MapNamedBufferRange(GLuint buffer, GLintptr offset, GLsizeiptr length, + GLbitfield access); + +void * GLAPIENTRY +_mesa_MapBuffer(GLenum target, GLenum access); + +void * GLAPIENTRY +_mesa_MapNamedBuffer(GLuint buffer, GLenum access); + + void GLAPIENTRY _mesa_FlushMappedBufferRange(GLenum target, GLintptr offset, GLsizeiptr length); diff --git a/src/mesa/main/tests/dispatch_sanity.cpp b/src/mesa/main/tests/dispatch_sanity.cpp index 9dcfecc..5632ac0 100644 --- a/src/mesa/main/tests/dispatch_sanity.cpp +++ b/src/mesa/main/tests/dispatch_sanity.cpp @@ -962,6 +962,8 @@ const struct function gl_core_functions_possible[] = { { "glCopyNamedBufferSubData", 45, -1 }, { "glClearNamedBufferData", 45, -1 }, { "glClearNamedBufferSubData", 45, -1 }, + { "glMapNamedBuffer", 45, -1 }, + { "glMapNamedBufferRange", 45, -1 }, { "glCreateTextures", 45, -1 }, { "glTextureStorage1D", 45, -1 }, { "glTextureStorage2D", 45, -1 }, -- 2.1.0 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div>