dlist: upload vertices in compile_vertex_list

Previously vertices were uploaded on-the-fly: each time
the position attribute was set, the newly added vertex
was copied to the mapped bo.

Replace this with a plain RAM buffer, and do the upload
at the end of compile_vertex_list.

This allows to remove the we-need-to-unmap-the-buffer-
before-drawing special case, but more importantly it
will allow to implement vertices deduplication in the
next commit.

Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11927>

src/mesa/vbo/vbo_save.c

@@ -68,6 +68,7 @@ void vbo_save_destroy( struct gl_context *ctx )
    }
    if (save->vertex_store) {
       _mesa_reference_buffer_object(ctx, &save->vertex_store->bufferobj, NULL);
+      free(save->vertex_store->buffer_in_ram);
       free(save->vertex_store);
       save->vertex_store = NULL;
    }

src/mesa/vbo/vbo_save.h

@@ -150,7 +150,8 @@ _vbo_save_get_vertex_count(const struct vbo_save_vertex_list *node)
 
 struct vbo_save_vertex_store {
    struct gl_buffer_object *bufferobj;
-   fi_type *buffer_map;
+   fi_type *buffer_in_ram;
+   GLuint buffer_in_ram_size;
    GLuint used;           /**< Number of 4-byte words used in buffer */
 };
 
@@ -170,7 +171,8 @@ void vbo_save_destroy(struct gl_context *ctx);
 /* save_loopback.c:
  */
 void _vbo_loopback_vertex_list(struct gl_context *ctx,
-                               const struct vbo_save_vertex_list* node);
+                               const struct vbo_save_vertex_list* node,
+                               fi_type *buffer);
 
 /* Callbacks:
  */
@@ -183,12 +185,4 @@ vbo_save_playback_vertex_list_loopback(struct gl_context *ctx, void *data);
 void
 vbo_save_api_init(struct vbo_save_context *save);
 
-fi_type *
-vbo_save_map_vertex_store(struct gl_context *ctx,
-                          struct vbo_save_vertex_store *vertex_store);
-
-void
-vbo_save_unmap_vertex_store(struct gl_context *ctx,
-                            struct vbo_save_vertex_store *vertex_store);
-
 #endif /* VBO_SAVE_H */

src/mesa/vbo/vbo_save_api.c

@@ -146,13 +146,15 @@ alloc_vertex_store(struct gl_context *ctx, int vertex_count)
     */
    vertex_store->bufferobj = ctx->Driver.NewBufferObject(ctx, VBO_BUF_ID);
    if (vertex_store->bufferobj) {
+      vertex_store->buffer_in_ram_size = size * sizeof(GLfloat);
+      vertex_store->buffer_in_ram = malloc(vertex_store->buffer_in_ram_size);
+      save->out_of_memory = vertex_store->buffer_in_ram == NULL;
       save->out_of_memory =
          !ctx->Driver.BufferData(ctx,
                                  GL_ARRAY_BUFFER_ARB,
                                  size * sizeof(GLfloat),
                                  NULL, GL_STATIC_DRAW_ARB,
-                                 GL_MAP_WRITE_BIT |
-                                 GL_DYNAMIC_STORAGE_BIT,
+                                 GL_MAP_WRITE_BIT,
                                  vertex_store->bufferobj);
    }
    else {
@@ -164,7 +166,6 @@ alloc_vertex_store(struct gl_context *ctx, int vertex_count)
       _mesa_install_save_vtxfmt(ctx, &save->vtxfmt_noop);
    }
 
-   vertex_store->buffer_map = NULL;
    vertex_store->used = 0;
 
    return vertex_store;
@@ -175,7 +176,7 @@ static void
 free_vertex_store(struct gl_context *ctx,
                   struct vbo_save_vertex_store *vertex_store)
 {
-   assert(!vertex_store->buffer_map);
+   free(vertex_store->buffer_in_ram);
 
    if (vertex_store->bufferobj) {
       _mesa_reference_buffer_object(ctx, &vertex_store->bufferobj, NULL);
@@ -185,65 +186,6 @@ free_vertex_store(struct gl_context *ctx,
 }
 
 
-fi_type *
-vbo_save_map_vertex_store(struct gl_context *ctx,
-                          struct vbo_save_vertex_store *vertex_store)
-{
-   const GLbitfield access = (GL_MAP_WRITE_BIT |
-                              GL_MAP_INVALIDATE_RANGE_BIT |
-                              GL_MAP_UNSYNCHRONIZED_BIT |
-                              GL_MAP_FLUSH_EXPLICIT_BIT |
-                              MESA_MAP_ONCE);
-
-   assert(vertex_store->bufferobj);
-   assert(!vertex_store->buffer_map);  /* the buffer should not be mapped */
-
-   if (vertex_store->bufferobj->Size > 0) {
-      /* Map the remaining free space in the VBO */
-      GLintptr offset = vertex_store->used * sizeof(GLfloat);
-      GLsizeiptr size = vertex_store->bufferobj->Size - offset;
-      fi_type *range = (fi_type *)
-         ctx->Driver.MapBufferRange(ctx, offset, size, access,
-                                    vertex_store->bufferobj,
-                                    MAP_INTERNAL);
-      if (range) {
-         /* compute address of start of whole buffer (needed elsewhere) */
-         vertex_store->buffer_map = range - vertex_store->used;
-         assert(vertex_store->buffer_map);
-         return range;
-      }
-      else {
-         vertex_store->buffer_map = NULL;
-         return NULL;
-      }
-   }
-   else {
-      /* probably ran out of memory for buffers */
-      return NULL;
-   }
-}
-
-
-void
-vbo_save_unmap_vertex_store(struct gl_context *ctx,
-                            struct vbo_save_vertex_store *vertex_store)
-{
-   if (vertex_store->bufferobj->Size > 0) {
-      GLintptr offset = 0;
-      GLsizeiptr length = vertex_store->used * sizeof(GLfloat)
-         - vertex_store->bufferobj->Mappings[MAP_INTERNAL].Offset;
-
-      /* Explicitly flush the region we wrote to */
-      ctx->Driver.FlushMappedBufferRange(ctx, offset, length,
-                                         vertex_store->bufferobj,
-                                         MAP_INTERNAL);
-
-      ctx->Driver.UnmapBuffer(ctx, vertex_store->bufferobj, MAP_INTERNAL);
-   }
-   vertex_store->buffer_map = NULL;
-}
-
-
 static struct vbo_save_primitive_store *
 alloc_prim_store(int prim_count)
 {
@@ -263,7 +205,7 @@ reset_counters(struct gl_context *ctx)
    struct vbo_save_context *save = &vbo_context(ctx)->save;
 
    save->prims = save->prim_store->prims + save->prim_store->used;
-   save->buffer_map = save->vertex_store->buffer_map + save->vertex_store->used;
+   save->buffer_map = save->vertex_store->buffer_in_ram + save->vertex_store->used;
 
    assert(save->buffer_map == save->buffer_ptr);
 
@@ -482,10 +424,6 @@ realloc_storage(struct gl_context *ctx, int prim_count, int vertex_count)
 {
    struct vbo_save_context *save = &vbo_context(ctx)->save;
    if (vertex_count >= 0) {
-      /* Unmap old store:
-       */
-      vbo_save_unmap_vertex_store(ctx, save->vertex_store);
-
       /* Release old reference:
        */
       free_vertex_store(ctx, save->vertex_store);
@@ -497,7 +435,7 @@ realloc_storage(struct gl_context *ctx, int prim_count, int vertex_count)
       /* Allocate and map new store:
        */
       save->vertex_store = alloc_vertex_store(ctx, vertex_count);
-      save->buffer_ptr = vbo_save_map_vertex_store(ctx, save->vertex_store);
+      save->buffer_ptr = save->vertex_store->buffer_in_ram + save->vertex_store->used;
       save->out_of_memory = save->buffer_ptr == NULL;
    }
 
@@ -545,7 +483,8 @@ compile_vertex_list(struct gl_context *ctx)
    }
    const GLsizei stride = save->vertex_size*sizeof(GLfloat);
    GLintptr buffer_offset =
-       (save->buffer_map - save->vertex_store->buffer_map) * sizeof(GLfloat);
+       (save->buffer_map - save->vertex_store->buffer_in_ram) * sizeof(GLfloat);
+   const GLintptr original_buffer_offset = buffer_offset;
    assert(old_offset <= buffer_offset);
    const GLintptr offset_diff = buffer_offset - old_offset;
    GLuint start_offset = 0;
@@ -798,6 +737,12 @@ compile_vertex_list(struct gl_context *ctx)
       node->cold->prim_count = 0;
    }
 
+   ctx->Driver.BufferSubData(ctx,
+                             original_buffer_offset,
+                             idx * save->vertex_size * sizeof(fi_type),
+                             &save->vertex_store->buffer_in_ram[original_buffer_offset / sizeof(float)],
+                             save->vertex_store->bufferobj);
+
    /* Prepare for DrawGallium */
    memset(&node->merged.info, 0, sizeof(struct pipe_draw_info));
    /* The other info fields will be updated in vbo_save_playback_vertex_list */
@@ -846,7 +791,7 @@ end:
       _glapi_set_dispatch(ctx->Exec);
 
       /* Note that the range of referenced vertices must be mapped already */
-      _vbo_loopback_vertex_list(ctx, node);
+      _vbo_loopback_vertex_list(ctx, node, save->vertex_store->buffer_in_ram);
 
       _glapi_set_dispatch(dispatch);
    }
@@ -860,7 +805,7 @@ end:
    }
    else {
       /* update buffer_ptr for next vertex */
-      save->buffer_ptr = save->vertex_store->buffer_map
+      save->buffer_ptr = save->vertex_store->buffer_in_ram
          + save->vertex_store->used;
    }
 
@@ -1882,7 +1827,7 @@ vbo_save_NewList(struct gl_context *ctx, GLuint list, GLenum mode)
    if (!save->vertex_store)
       save->vertex_store = alloc_vertex_store(ctx, 0);
 
-   save->buffer_ptr = vbo_save_map_vertex_store(ctx, save->vertex_store);
+   save->buffer_ptr = save->vertex_store->buffer_in_ram + save->vertex_store->used;
 
    reset_vertex(ctx);
    reset_counters(ctx);
@@ -1921,8 +1866,6 @@ vbo_save_EndList(struct gl_context *ctx)
       _mesa_install_save_vtxfmt(ctx, &ctx->ListState.ListVtxfmt);
    }
 
-   vbo_save_unmap_vertex_store(ctx, save->vertex_store);
-
    assert(save->vertex_size == 0);
 }
 

src/mesa/vbo/vbo_save_draw.c

@@ -147,11 +147,12 @@ loopback_vertex_list(struct gl_context *ctx,
                      const struct vbo_save_vertex_list *list)
 {
    struct gl_buffer_object *bo = list->VAO[0]->BufferBinding[0].BufferObj;
-   ctx->Driver.MapBufferRange(ctx, 0, bo->Size, GL_MAP_READ_BIT, /* ? */
-                              bo, MAP_INTERNAL);
+   void *buffer = ctx->Driver.MapBufferRange(ctx, 0, bo->Size, GL_MAP_READ_BIT, /* ? */
+                                             bo, MAP_INTERNAL);
 
-   /* Note that the range of referenced vertices must be mapped already */
-   _vbo_loopback_vertex_list(ctx, list);
+   /* TODO: in this case, we shouldn't create a bo at all and instead keep
+    * the in-RAM buffer. */
+   _vbo_loopback_vertex_list(ctx, list, buffer);
 
    ctx->Driver.UnmapBuffer(ctx, bo, MAP_INTERNAL);
 }
@@ -162,21 +163,6 @@ vbo_save_playback_vertex_list_loopback(struct gl_context *ctx, void *data)
 {
    const struct vbo_save_vertex_list *node =
       (const struct vbo_save_vertex_list *) data;
-   struct vbo_context *vbo = vbo_context(ctx);
-   struct vbo_save_context *save = &vbo->save;
-   GLboolean remap_vertex_store = GL_FALSE;
-
-   if (save->vertex_store && save->vertex_store->buffer_map) {
-      /* The vertex store is currently mapped but we're about to replay
-       * a display list.  This can happen when a nested display list is
-       * being build with GL_COMPILE_AND_EXECUTE.
-       * We never want to have mapped vertex buffers when we're drawing.
-       * Unmap the vertex store, execute the list, then remap the vertex
-       * store.
-       */
-      vbo_save_unmap_vertex_store(ctx, save->vertex_store);
-      remap_vertex_store = GL_TRUE;
-   }
 
    FLUSH_FOR_DRAW(ctx);
 
@@ -186,17 +172,12 @@ vbo_save_playback_vertex_list_loopback(struct gl_context *ctx, void *data)
        */
       _mesa_error(ctx, GL_INVALID_OPERATION,
                   "draw operation inside glBegin/End");
-      goto end;
+      return;
    }
    /* Various degenerate cases: translate into immediate mode
     * calls rather than trying to execute in place.
     */
    loopback_vertex_list(ctx, node);
-
-end:
-   if (remap_vertex_store) {
-      save->buffer_ptr = vbo_save_map_vertex_store(ctx, save->vertex_store);
-   }
 }
 
 /**

src/mesa/vbo/vbo_save_loopback.c

@@ -146,7 +146,8 @@ append_attr(GLuint *nr, struct loopback_attr la[], int i, int shift,
 
 void
 _vbo_loopback_vertex_list(struct gl_context *ctx,
-                          const struct vbo_save_vertex_list* node)
+                          const struct vbo_save_vertex_list* node,
+                          fi_type *buffer)
 {
    struct loopback_attr la[VBO_ATTRIB_MAX];
    GLuint nr = 0;
@@ -177,32 +178,13 @@ _vbo_loopback_vertex_list(struct gl_context *ctx,
 
    const GLuint wrap_count = node->cold->wrap_count;
    const GLuint stride = _vbo_save_get_stride(node);
-   const GLubyte *buffer = NULL;
-   if (0 < nr) {
-      /* Compute the minimal offset into the vertex buffer object */
-      GLuint offset = ~0u;
-      for (GLuint i = 0; i < nr; ++i)
-         offset = MIN2(offset, la[i].offset);
-      for (GLuint i = 0; i < nr; ++i)
-         la[i].offset -= offset;
-
-      /* Get the mapped base pointer, assert sufficient mapping */
-      struct gl_buffer_object *bufferobj = vao->BufferBinding[0].BufferObj;
-      assert(bufferobj && bufferobj->Mappings[MAP_INTERNAL].Pointer);
-      buffer = bufferobj->Mappings[MAP_INTERNAL].Pointer;
-      assert(bufferobj->Mappings[MAP_INTERNAL].Offset
-             <= vao->BufferBinding[0].Offset + offset
-             + stride*(_vbo_save_get_min_index(node) + wrap_count));
-      buffer += vao->BufferBinding[0].Offset + offset
-         - bufferobj->Mappings[MAP_INTERNAL].Offset;
-      assert(stride*(_vbo_save_get_vertex_count(node) - wrap_count)
-             <= bufferobj->Mappings[MAP_INTERNAL].Length);
-   }
 
    /* Replay the primitives */
    const struct _mesa_prim *prims = node->cold->prims;
    const GLuint prim_count = node->cold->prim_count;
+
    for (GLuint i = 0; i < prim_count; i++) {
-      loopback_prim(ctx, buffer, &prims[i], wrap_count, stride, la, nr);
+      loopback_prim(ctx, (GLubyte*)buffer + vao->BufferBinding[0].Offset,
+                    &prims[i], wrap_count, stride, la, nr);
    }
 }