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.
On Wed, Jan 21, 2015 at 5:40 PM, Laura Ekstrand <la...@jlekstrand.net> wrote: > --- > 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> > + > <!-- Texture object functions --> > > <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 > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev