From 28a2ad7ddf76702a5de56a7bc0d8754b7dbd66a0 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Marek=20Ol=C5=A1=C3=A1k?= Date: Thu, 20 Feb 2020 19:28:56 -0500 Subject: [PATCH] glthread: track for each VAO whether the user has set a user pointer This commit mainly adds basic infrastructure for tracking vertex array state. If glthread gets a non-VBO pointer, this commit delays disabling glthread until glDraw is called. The next will change that to "sync" instead of "disable". Reviewed-by: Timothy Arceri Part-of: --- src/mapi/glapi/gen/ARB_base_instance.xml | 3 +- src/mapi/glapi/gen/ARB_draw_instanced.xml | 3 +- .../glapi/gen/ARB_indirect_parameters.xml | 7 +- .../glapi/gen/ARB_vertex_array_object.xml | 8 +- .../glapi/gen/ARB_vertex_attrib_64bit.xml | 2 +- .../glapi/gen/EXT_direct_state_access.xml | 2 +- src/mapi/glapi/gen/GL3x.xml | 2 +- src/mapi/glapi/gen/es_EXT.xml | 2 +- src/mapi/glapi/gen/gl_API.xml | 38 ++--- src/mesa/Makefile.sources | 1 + src/mesa/main/glthread.c | 19 +++ src/mesa/main/glthread.h | 22 ++- src/mesa/main/glthread_varray.c | 144 ++++++++++++++++++ src/mesa/main/marshal.c | 2 +- src/mesa/main/marshal.h | 49 ++---- src/mesa/meson.build | 1 + 16 files changed, 237 insertions(+), 68 deletions(-) create mode 100644 src/mesa/main/glthread_varray.c diff --git a/src/mapi/glapi/gen/ARB_base_instance.xml b/src/mapi/glapi/gen/ARB_base_instance.xml index 92892c29078..22c57167fd5 100644 --- a/src/mapi/glapi/gen/ARB_base_instance.xml +++ b/src/mapi/glapi/gen/ARB_base_instance.xml @@ -8,7 +8,8 @@ - + diff --git a/src/mapi/glapi/gen/ARB_draw_instanced.xml b/src/mapi/glapi/gen/ARB_draw_instanced.xml index 8d7fd6301b3..e450f04a8e1 100644 --- a/src/mapi/glapi/gen/ARB_draw_instanced.xml +++ b/src/mapi/glapi/gen/ARB_draw_instanced.xml @@ -8,7 +8,8 @@ - + diff --git a/src/mapi/glapi/gen/ARB_indirect_parameters.xml b/src/mapi/glapi/gen/ARB_indirect_parameters.xml index 20de9057707..3938bd643bb 100644 --- a/src/mapi/glapi/gen/ARB_indirect_parameters.xml +++ b/src/mapi/glapi/gen/ARB_indirect_parameters.xml @@ -8,7 +8,8 @@ - + @@ -16,7 +17,9 @@ - + + diff --git a/src/mapi/glapi/gen/ARB_vertex_array_object.xml b/src/mapi/glapi/gen/ARB_vertex_array_object.xml index befa4c91c63..e149997d05c 100644 --- a/src/mapi/glapi/gen/ARB_vertex_array_object.xml +++ b/src/mapi/glapi/gen/ARB_vertex_array_object.xml @@ -11,16 +11,18 @@ + marshal_call_after="_mesa_glthread_BindVertexArray(ctx, array);"> - + - + diff --git a/src/mapi/glapi/gen/ARB_vertex_attrib_64bit.xml b/src/mapi/glapi/gen/ARB_vertex_attrib_64bit.xml index d96729be3ac..f75387bf8fe 100644 --- a/src/mapi/glapi/gen/ARB_vertex_attrib_64bit.xml +++ b/src/mapi/glapi/gen/ARB_vertex_attrib_64bit.xml @@ -52,7 +52,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> diff --git a/src/mapi/glapi/gen/EXT_direct_state_access.xml b/src/mapi/glapi/gen/EXT_direct_state_access.xml index 8e68ec38b99..0fe9ebcd647 100644 --- a/src/mapi/glapi/gen/EXT_direct_state_access.xml +++ b/src/mapi/glapi/gen/EXT_direct_state_access.xml @@ -658,7 +658,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> diff --git a/src/mapi/glapi/gen/GL3x.xml b/src/mapi/glapi/gen/GL3x.xml index 4bf7c132560..16035adc01d 100644 --- a/src/mapi/glapi/gen/GL3x.xml +++ b/src/mapi/glapi/gen/GL3x.xml @@ -258,7 +258,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> diff --git a/src/mapi/glapi/gen/es_EXT.xml b/src/mapi/glapi/gen/es_EXT.xml index c30ad56c75d..9dc1444bafe 100644 --- a/src/mapi/glapi/gen/es_EXT.xml +++ b/src/mapi/glapi/gen/es_EXT.xml @@ -320,7 +320,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml index 256d0f3aa15..6de88c6d7fe 100644 --- a/src/mapi/glapi/gen/gl_API.xml +++ b/src/mapi/glapi/gen/gl_API.xml @@ -3166,7 +3166,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> @@ -3179,7 +3179,8 @@ - + @@ -3197,7 +3198,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> @@ -3216,7 +3217,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> @@ -3232,7 +3233,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> @@ -3241,7 +3242,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> @@ -3251,7 +3252,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> @@ -4747,14 +4748,15 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> - + @@ -4887,7 +4889,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> @@ -5844,7 +5846,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> @@ -9242,7 +9244,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> @@ -9258,7 +9260,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> @@ -9271,7 +9273,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> @@ -9280,7 +9282,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> @@ -9289,7 +9291,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> @@ -9299,7 +9301,7 @@ + marshal_call_after="_mesa_glthread_AttribPointer(ctx);"> @@ -11390,7 +11392,7 @@ + marshal_fail="_mesa_glthread_is_non_vbo_draw_arrays(ctx)"> diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources index f55ae3de302..b09c019546b 100644 --- a/src/mesa/Makefile.sources +++ b/src/mesa/Makefile.sources @@ -125,6 +125,7 @@ MAIN_FILES = \ main/glspirv.h \ main/glthread.c \ main/glthread.h \ + main/glthread_varray.c \ main/glheader.h \ main/hash.c \ main/hash.h \ diff --git a/src/mesa/main/glthread.c b/src/mesa/main/glthread.c index af4eb15cb4d..4288c9aedbb 100644 --- a/src/mesa/main/glthread.c +++ b/src/mesa/main/glthread.c @@ -35,6 +35,7 @@ #include "main/mtypes.h" #include "main/glthread.h" #include "main/marshal.h" +#include "main/hash.h" #include "util/u_atomic.h" #include "util/u_thread.h" @@ -85,8 +86,17 @@ _mesa_glthread_init(struct gl_context *ctx) return; } + glthread->VAOs = _mesa_NewHashTable(); + if (!glthread->VAOs) { + util_queue_destroy(&glthread->queue); + free(glthread); + return; + } + glthread->CurrentVAO = &glthread->DefaultVAO; + ctx->MarshalExec = _mesa_create_marshal_table(ctx); if (!ctx->MarshalExec) { + _mesa_DeleteHashTable(glthread->VAOs); util_queue_destroy(&glthread->queue); free(glthread); return; @@ -110,6 +120,12 @@ _mesa_glthread_init(struct gl_context *ctx) util_queue_fence_destroy(&fence); } +static void +free_vao(GLuint key, void *data, void *userData) +{ + free(data); +} + void _mesa_glthread_destroy(struct gl_context *ctx) { @@ -124,6 +140,9 @@ _mesa_glthread_destroy(struct gl_context *ctx) for (unsigned i = 0; i < MARSHAL_MAX_BATCHES; i++) util_queue_fence_destroy(&glthread->batches[i].fence); + _mesa_HashDeleteAll(glthread->VAOs, free_vao, NULL); + _mesa_DeleteHashTable(glthread->VAOs); + free(glthread); ctx->GLThread = NULL; diff --git a/src/mesa/main/glthread.h b/src/mesa/main/glthread.h index 5e99602b418..d4a680ab038 100644 --- a/src/mesa/main/glthread.h +++ b/src/mesa/main/glthread.h @@ -46,9 +46,17 @@ #include #include #include "util/u_queue.h" +#include "GL/gl.h" enum marshal_dispatch_cmd_id; struct gl_context; +struct _mesa_HashTable; + +struct glthread_vao { + GLuint Name; + bool HasUserPointer; + bool IndexBufferIsUserPointer; +}; /** A single batch of commands queued up for execution. */ struct glthread_batch @@ -83,6 +91,12 @@ struct glthread_state /** Index of the batch being filled and about to be submitted. */ unsigned next; + /** Vertex Array objects tracked by glthread independently of Mesa. */ + struct _mesa_HashTable *VAOs; + struct glthread_vao *CurrentVAO; + struct glthread_vao *LastLookedUpVAO; + struct glthread_vao DefaultVAO; + /** * Tracks on the main thread side whether the current vertex array binding * is in a VBO. @@ -93,7 +107,6 @@ struct glthread_state * Tracks on the main thread side whether the current element array (index * buffer) binding is in a VBO. */ - bool element_array_is_vbo; bool draw_indirect_buffer_is_vbo; }; @@ -106,4 +119,11 @@ void _mesa_glthread_flush_batch(struct gl_context *ctx); void _mesa_glthread_finish(struct gl_context *ctx); void _mesa_glthread_finish_before(struct gl_context *ctx, const char *func); +void _mesa_glthread_BindVertexArray(struct gl_context *ctx, GLuint id); +void _mesa_glthread_DeleteVertexArrays(struct gl_context *ctx, + GLsizei n, const GLuint *ids); +void _mesa_glthread_GenVertexArrays(struct gl_context *ctx, + GLsizei n, GLuint *arrays); +void _mesa_glthread_AttribPointer(struct gl_context *ctx); + #endif /* _GLTHREAD_H*/ diff --git a/src/mesa/main/glthread_varray.c b/src/mesa/main/glthread_varray.c new file mode 100644 index 00000000000..8786f0f2d09 --- /dev/null +++ b/src/mesa/main/glthread_varray.c @@ -0,0 +1,144 @@ +/* + * Copyright © 2020 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +/* This implements vertex array state tracking for glthread. It's separate + * from the rest of Mesa. Only minimum functionality is implemented here + * to serve glthread. + */ + +#include "main/glthread.h" +#include "main/mtypes.h" +#include "main/hash.h" +#include "main/dispatch.h" + +/* TODO: + * - Implement better tracking of user pointers + * - These can unbind user pointers: + * ARB_vertex_attrib_binding + * ARB_direct_state_access + * EXT_direct_state_access + */ + +static struct glthread_vao * +lookup_vao(struct gl_context *ctx, GLuint id) +{ + struct glthread_state *glthread = ctx->GLThread; + struct glthread_vao *vao; + + assert(id != 0); + + if (glthread->LastLookedUpVAO && + glthread->LastLookedUpVAO->Name == id) { + vao = glthread->LastLookedUpVAO; + } else { + vao = _mesa_HashLookupLocked(glthread->VAOs, id); + if (!vao) + return NULL; + + glthread->LastLookedUpVAO = vao; + } + + return vao; +} + +void +_mesa_glthread_BindVertexArray(struct gl_context *ctx, GLuint id) +{ + struct glthread_state *glthread = ctx->GLThread; + + if (id == 0) { + glthread->CurrentVAO = &glthread->DefaultVAO; + } else { + struct glthread_vao *vao = lookup_vao(ctx, id); + + if (vao) + glthread->CurrentVAO = vao; + } +} + +void +_mesa_glthread_DeleteVertexArrays(struct gl_context *ctx, + GLsizei n, const GLuint *ids) +{ + struct glthread_state *glthread = ctx->GLThread; + + if (!ids) + return; + + for (int i = 0; i < n; i++) { + /* IDs equal to 0 should be silently ignored. */ + if (!ids[i]) + continue; + + struct glthread_vao *vao = lookup_vao(ctx, ids[i]); + if (!vao) + continue; + + /* If the array object is currently bound, the spec says "the binding + * for that object reverts to zero and the default vertex array + * becomes current." + */ + if (glthread->CurrentVAO == vao) + glthread->CurrentVAO = &glthread->DefaultVAO; + + if (glthread->LastLookedUpVAO == vao) + glthread->LastLookedUpVAO = NULL; + + /* The ID is immediately freed for re-use */ + _mesa_HashRemoveLocked(glthread->VAOs, vao->Name); + free(vao); + } +} + +void +_mesa_glthread_GenVertexArrays(struct gl_context *ctx, + GLsizei n, GLuint *arrays) +{ + struct glthread_state *glthread = ctx->GLThread; + + if (!arrays) + return; + + /* The IDs have been generated at this point. Create VAOs for glthread. */ + for (int i = 0; i < n; i++) { + GLuint id = arrays[i]; + struct glthread_vao *vao; + + vao = malloc(sizeof(*vao)); + if (!vao) + continue; /* Is that all we can do? */ + + vao->Name = id; + vao->HasUserPointer = false; + _mesa_HashInsertLocked(glthread->VAOs, id, vao); + } +} + +void +_mesa_glthread_AttribPointer(struct gl_context *ctx) +{ + struct glthread_state *glthread = ctx->GLThread; + + if (ctx->API != API_OPENGL_CORE && !glthread->vertex_array_is_vbo) + glthread->CurrentVAO->HasUserPointer = true; +} diff --git a/src/mesa/main/marshal.c b/src/mesa/main/marshal.c index ce9bbf8139f..ce91d6a8f66 100644 --- a/src/mesa/main/marshal.c +++ b/src/mesa/main/marshal.c @@ -182,7 +182,7 @@ track_vbo_binding(struct gl_context *ctx, GLenum target, GLuint buffer) * vertex array object instead of the context, so this would need to * change on vertex array object updates. */ - glthread->element_array_is_vbo = (buffer != 0); + glthread->CurrentVAO->IndexBufferIsUserPointer = buffer != 0; break; case GL_DRAW_INDIRECT_BUFFER: glthread->draw_indirect_buffer_is_vbo = buffer != 0; diff --git a/src/mesa/main/marshal.h b/src/mesa/main/marshal.h index 78f6fbc34f7..c53c060f593 100644 --- a/src/mesa/main/marshal.h +++ b/src/mesa/main/marshal.h @@ -74,28 +74,25 @@ _mesa_glthread_allocate_command(struct gl_context *ctx, } /** - * Instead of conditionally handling marshaling previously-bound user vertex - * array data in draw calls (deprecated and removed in GL core), we just - * disable threading at the point where the user sets a user vertex array. + * Instead of conditionally handling marshaling immediate index data in draw + * calls (deprecated and removed in GL core), we just disable threading. */ static inline bool -_mesa_glthread_is_non_vbo_vertex_attrib_pointer(const struct gl_context *ctx) +_mesa_glthread_is_non_vbo_draw_elements(const struct gl_context *ctx) { struct glthread_state *glthread = ctx->GLThread; - return ctx->API != API_OPENGL_CORE && !glthread->vertex_array_is_vbo; + return ctx->API != API_OPENGL_CORE && + (glthread->CurrentVAO->IndexBufferIsUserPointer || + glthread->CurrentVAO->HasUserPointer); } -/** - * Instead of conditionally handling marshaling immediate index data in draw - * calls (deprecated and removed in GL core), we just disable threading. - */ static inline bool -_mesa_glthread_is_non_vbo_draw_elements(const struct gl_context *ctx) +_mesa_glthread_is_non_vbo_draw_arrays(const struct gl_context *ctx) { struct glthread_state *glthread = ctx->GLThread; - return ctx->API != API_OPENGL_CORE && !glthread->element_array_is_vbo; + return ctx->API != API_OPENGL_CORE && glthread->CurrentVAO->HasUserPointer; } static inline bool @@ -104,7 +101,8 @@ _mesa_glthread_is_non_vbo_draw_arrays_indirect(const struct gl_context *ctx) struct glthread_state *glthread = ctx->GLThread; return ctx->API != API_OPENGL_CORE && - !glthread->draw_indirect_buffer_is_vbo; + (!glthread->draw_indirect_buffer_is_vbo || + glthread->CurrentVAO->HasUserPointer ); } static inline bool @@ -114,7 +112,8 @@ _mesa_glthread_is_non_vbo_draw_elements_indirect(const struct gl_context *ctx) return ctx->API != API_OPENGL_CORE && (!glthread->draw_indirect_buffer_is_vbo || - !glthread->element_array_is_vbo); + glthread->CurrentVAO->IndexBufferIsUserPointer || + glthread->CurrentVAO->HasUserPointer); } #define DEBUG_MARSHAL_PRINT_CALLS 0 @@ -151,30 +150,6 @@ debug_print_marshal(const char *func) struct _glapi_table * _mesa_create_marshal_table(const struct gl_context *ctx); - -/** - * Checks whether we're on a compat context for code-generated - * glBindVertexArray(). - * - * In order to decide whether a draw call uses only VBOs for vertex and index - * buffers, we track the current vertex and index buffer bindings by - * glBindBuffer(). However, the index buffer binding is stored in the vertex - * array as opposed to the context. If we were to accurately track whether - * the index buffer was a user pointer ot not, we'd have to track it per - * vertex array, which would mean synchronizing with the client thread and - * looking into the hash table to find the actual vertex array object. That's - * more tracking than we'd like to do in the main thread, if possible. - * - * Instead, just punt for now and disable threading on apps using vertex - * arrays and compat contexts. Apps using vertex arrays can probably use a - * core context. - */ -static inline bool -_mesa_glthread_is_compat_bind_vertex_array(const struct gl_context *ctx) -{ - return ctx->API != API_OPENGL_CORE; -} - struct marshal_cmd_ShaderSource; struct marshal_cmd_BindBuffer; struct marshal_cmd_BufferData; diff --git a/src/mesa/meson.build b/src/mesa/meson.build index 1060cfe3388..2db8bbe8e9c 100644 --- a/src/mesa/meson.build +++ b/src/mesa/meson.build @@ -167,6 +167,7 @@ files_libmesa_common = files( 'main/glspirv.h', 'main/glthread.c', 'main/glthread.h', + 'main/glthread_varray.c', 'main/glheader.h', 'main/hash.c', 'main/hash.h', -- 2.30.2