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 <pierre-eric.pelloux-prayer@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20624>

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

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
 }

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,

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);
-      }
    }
 }
 

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);
+}

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 */

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;

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);