From 616d595d18d54c8e39e64386a5a2ac2be8e5fef9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Ol=C5=A1=C3=A1k?= Date: Tue, 27 Dec 2022 02:37:20 -0500 Subject: [PATCH] glthread: don't restore non-VBO vertex arrays after all draws glthread takes care of all uploads, so it's OK to leave uploaded VBOs bound. The only thing that will be wrong is the bound vertex buffer returned by glGet, but the only case when that would be wrong is when an app that doesn't use VBOs queries the current VBO. That never happens. However, this adds code to unbind all internal VBOs for the case when glthread is abruptly disabled (e.g. for GL_DEBUG_OUTPUT_SYNCHRONOUS). Acked-by: Pierre-Eric Pelloux-Prayer Part-of: --- src/mesa/main/glthread.c | 6 +++ src/mesa/main/glthread.h | 2 +- src/mesa/main/glthread_bufferobj.c | 1 + src/mesa/main/glthread_draw.c | 76 +++++------------------------- src/mesa/main/glthread_varray.c | 34 +++++++++++++ src/mesa/main/mtypes.h | 1 + src/mesa/main/varray.c | 15 +----- src/mesa/main/varray.h | 3 +- 8 files changed, 57 insertions(+), 81 deletions(-) diff --git a/src/mesa/main/glthread.c b/src/mesa/main/glthread.c index 0fa42de1d51..ca0f638361a 100644 --- a/src/mesa/main/glthread.c +++ b/src/mesa/main/glthread.c @@ -277,6 +277,12 @@ void _mesa_glthread_disable(struct gl_context *ctx) if (_glapi_get_dispatch() == ctx->MarshalExec) { _glapi_set_dispatch(ctx->CurrentClientDispatch); } + + /* Unbind VBOs in all VAOs that glthread bound for non-VBO vertex uploads + * to restore original states. + */ + if (ctx->API != API_OPENGL_CORE) + _mesa_glthread_unbind_uploaded_vbos(ctx); } void diff --git a/src/mesa/main/glthread.h b/src/mesa/main/glthread.h index 9099567aa8c..55e57c2fbb6 100644 --- a/src/mesa/main/glthread.h +++ b/src/mesa/main/glthread.h @@ -88,7 +88,6 @@ union gl_vertex_format_user { struct glthread_attrib_binding { struct gl_buffer_object *buffer; /**< where non-VBO data was uploaded */ int offset; /**< offset to uploaded non-VBO data */ - const void *original_pointer; /**< restore this pointer after the draw */ }; struct glthread_attrib { @@ -372,6 +371,7 @@ void _mesa_glthread_ProgramChanged(struct gl_context *ctx); void _mesa_glthread_UnrollDrawElements(struct gl_context *ctx, GLenum mode, GLsizei count, GLenum type, const GLvoid *indices, GLint basevertex); +void _mesa_glthread_unbind_uploaded_vbos(struct gl_context *ctx); #ifdef __cplusplus } diff --git a/src/mesa/main/glthread_bufferobj.c b/src/mesa/main/glthread_bufferobj.c index 609cb0ffcad..58e89aba962 100644 --- a/src/mesa/main/glthread_bufferobj.c +++ b/src/mesa/main/glthread_bufferobj.c @@ -38,6 +38,7 @@ new_upload_buffer(struct gl_context *ctx, GLsizeiptr size, uint8_t **ptr) return NULL; obj->Immutable = true; + obj->GLThreadInternal = true; if (!_mesa_bufferobj_data(ctx, GL_ARRAY_BUFFER, size, NULL, GL_WRITE_ONLY, diff --git a/src/mesa/main/glthread_draw.c b/src/mesa/main/glthread_draw.c index 1c3b13a6c38..6ea42d91855 100644 --- a/src/mesa/main/glthread_draw.c +++ b/src/mesa/main/glthread_draw.c @@ -215,7 +215,6 @@ upload_vertices(struct gl_context *ctx, unsigned user_buffer_mask, buffers[num_buffers].buffer = upload_buffer; buffers[num_buffers].offset = upload_offset - start; - buffers[num_buffers].original_pointer = ptr; num_buffers++; } @@ -276,7 +275,6 @@ upload_vertices(struct gl_context *ctx, unsigned user_buffer_mask, buffers[num_buffers].buffer = upload_buffer; buffers[num_buffers].offset = upload_offset - offset; - buffers[num_buffers].original_pointer = ptr; num_buffers++; } @@ -389,10 +387,8 @@ _mesa_unmarshal_DrawArraysUserBuf(struct gl_context *ctx, (const struct glthread_attrib_binding *)(cmd + 1); /* Bind uploaded buffers if needed. */ - if (user_buffer_mask) { - _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask, - false); - } + if (user_buffer_mask) + _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask); const GLenum mode = cmd->mode; const GLint first = cmd->first; @@ -405,12 +401,6 @@ _mesa_unmarshal_DrawArraysUserBuf(struct gl_context *ctx, (mode, first, count, instance_count, baseinstance)); ctx->DrawID = 0; - - /* Restore states. */ - if (user_buffer_mask) { - _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask, - true); - } return cmd->cmd_base.cmd_size; } @@ -540,19 +530,11 @@ _mesa_unmarshal_MultiDrawArraysUserBuf(struct gl_context *ctx, (const struct glthread_attrib_binding *)variable_data; /* Bind uploaded buffers if needed. */ - if (user_buffer_mask) { - _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask, - false); - } + if (user_buffer_mask) + _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask); CALL_MultiDrawArrays(ctx->CurrentServerDispatch, (mode, first, count, draw_count)); - - /* Restore states. */ - if (user_buffer_mask) { - _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask, - true); - } return cmd->cmd_base.cmd_size; } @@ -636,20 +618,11 @@ _mesa_marshal_MultiDrawArrays(GLenum mode, const GLint *first, /* The call is too large, so sync and execute the unmarshal code here. */ _mesa_glthread_finish_before(ctx, "MultiDrawArrays"); - if (user_buffer_mask) { - _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask, - false); - } + if (user_buffer_mask) + _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask); CALL_MultiDrawArrays(ctx->CurrentServerDispatch, (mode, first, count, draw_count)); - - /* Restore states. */ - if (user_buffer_mask) { - /* TODO: remove this after glthread takes over all uploading */ - _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask, - true); - } } } @@ -804,10 +777,8 @@ _mesa_unmarshal_DrawElementsUserBuf(struct gl_context *ctx, (const struct glthread_attrib_binding *)(cmd + 1); /* Bind uploaded buffers if needed. */ - if (user_buffer_mask) { - _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask, - false); - } + if (user_buffer_mask) + _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask); /* Draw. */ const GLenum mode = cmd->mode; @@ -826,12 +797,6 @@ _mesa_unmarshal_DrawElementsUserBuf(struct gl_context *ctx, baseinstance)); ctx->DrawID = 0; _mesa_reference_buffer_object(ctx, &index_buffer, NULL); - - /* Restore states. */ - if (user_buffer_mask) { - _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask, - true); - } return cmd->cmd_base.cmd_size; } @@ -1057,10 +1022,8 @@ _mesa_unmarshal_MultiDrawElementsUserBuf(struct gl_context *ctx, (const struct glthread_attrib_binding *)variable_data; /* Bind uploaded buffers if needed. */ - if (user_buffer_mask) { - _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask, - false); - } + if (user_buffer_mask) + _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask); /* Draw. */ const GLenum mode = cmd->mode; @@ -1071,12 +1034,6 @@ _mesa_unmarshal_MultiDrawElementsUserBuf(struct gl_context *ctx, ((GLintptr)index_buffer, mode, count, type, indices, draw_count, basevertex)); _mesa_reference_buffer_object(ctx, &index_buffer, NULL); - - /* Restore states. */ - if (user_buffer_mask) { - _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask, - true); - } return cmd->cmd_base.cmd_size; } @@ -1126,23 +1083,14 @@ multi_draw_elements_async(struct gl_context *ctx, GLenum mode, _mesa_glthread_finish_before(ctx, "DrawElements"); /* Bind uploaded buffers if needed. */ - if (user_buffer_mask) { - _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask, - false); - } + if (user_buffer_mask) + _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask); /* Draw. */ CALL_MultiDrawElementsUserBuf(ctx->CurrentServerDispatch, ((GLintptr)index_buffer, mode, count, type, indices, draw_count, basevertex)); _mesa_reference_buffer_object(ctx, &index_buffer, NULL); - - /* Restore states. */ - /* TODO: remove this after glthread takes over all uploading */ - if (user_buffer_mask) { - _mesa_InternalBindVertexBuffers(ctx, buffers, user_buffer_mask, - true); - } } } diff --git a/src/mesa/main/glthread_varray.c b/src/mesa/main/glthread_varray.c index ca58ca07111..98b3027869e 100644 --- a/src/mesa/main/glthread_varray.c +++ b/src/mesa/main/glthread_varray.c @@ -739,3 +739,37 @@ _mesa_glthread_InterleavedArrays(struct gl_context *ctx, GLenum format, 0, 0, 0), stride, (GLubyte *) pointer + layout.voffset); } + +static void +unbind_uploaded_vbos(void *_vao, void *_ctx) +{ + struct gl_context *ctx = _ctx; + struct gl_vertex_array_object *vao = _vao; + + assert(ctx->API != API_OPENGL_CORE); + + for (unsigned i = 0; i < ARRAY_SIZE(vao->BufferBinding); i++) { + if (vao->BufferBinding[i].BufferObj && + vao->BufferBinding[i].BufferObj->GLThreadInternal) { + /* We don't need to restore the user pointer because it's never + * overwritten. When we bind a VBO internally, the user pointer + * in gl_array_attribute::Ptr becomes ignored and unchanged. + */ + _mesa_bind_vertex_buffer(ctx, vao, i, NULL, 0, + vao->BufferBinding[i].Stride, false, false); + } + } +} + +/* Unbind VBOs in all VAOs that glthread bound for non-VBO vertex uploads + * to restore original states. + */ +void +_mesa_glthread_unbind_uploaded_vbos(struct gl_context *ctx) +{ + assert(ctx->API != API_OPENGL_CORE); + + /* Iterate over all VAOs. */ + _mesa_HashWalk(ctx->Array.Objects, unbind_uploaded_vbos, ctx); + unbind_uploaded_vbos(ctx->Array.DefaultVAO, ctx); +} diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index 6e9b437c5a6..2333def2ac3 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -1465,6 +1465,7 @@ struct gl_buffer_object bool DeletePending:1; /**< true if buffer object is removed from the hash */ bool Immutable:1; /**< GL_ARB_buffer_storage */ bool HandleAllocated:1; /**< GL_ARB_bindless_texture */ + bool GLThreadInternal:1; /**< Created by glthread. */ GLenum16 Usage; /**< GL_STREAM_DRAW_ARB, GL_STREAM_READ_ARB, etc. */ GLchar *Label; /**< GL_KHR_debug */ GLsizeiptrARB Size; /**< Size of buffer storage in bytes */ diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c index 2b9f347fc5b..82ea4cad4d3 100644 --- a/src/mesa/main/varray.c +++ b/src/mesa/main/varray.c @@ -3543,24 +3543,11 @@ _mesa_BindVertexBuffers(GLuint first, GLsizei count, const GLuint *buffers, void _mesa_InternalBindVertexBuffers(struct gl_context *ctx, const struct glthread_attrib_binding *buffers, - GLbitfield buffer_mask, - GLboolean restore_pointers) + GLbitfield buffer_mask) { struct gl_vertex_array_object *vao = ctx->Array.VAO; unsigned param_index = 0; - if (restore_pointers) { - while (buffer_mask) { - unsigned i = u_bit_scan(&buffer_mask); - - _mesa_bind_vertex_buffer(ctx, vao, i, NULL, - (GLintptr)buffers[param_index].original_pointer, - vao->BufferBinding[i].Stride, false, false); - param_index++; - } - return; - } - while (buffer_mask) { unsigned i = u_bit_scan(&buffer_mask); struct gl_buffer_object *buf = buffers[param_index].buffer; diff --git a/src/mesa/main/varray.h b/src/mesa/main/varray.h index 07cfa23920a..903f89fe816 100644 --- a/src/mesa/main/varray.h +++ b/src/mesa/main/varray.h @@ -156,8 +156,7 @@ _mesa_primitive_restart_index(const struct gl_context *ctx, void _mesa_InternalBindVertexBuffers(struct gl_context *ctx, const struct glthread_attrib_binding *buffers, - GLbitfield buffer_mask, - GLboolean restore_pointers); + GLbitfield buffer_mask); extern void _mesa_print_arrays(struct gl_context *ctx);