GWN: Vertex Buffer refactor.

We now alloc a vbo id on creation and let OpenGL manage its memory directly.
We use glMapBuffer to get this memory location.

This enables us to reuse and modify any vertex buffer directly without
destroying it with its associated Batches.

This commit does not really improve performance but will let us implement
more optimizations in the future.

We can also resize the buffer even if this can be slow if we need to keep
the existing data.

The addition of the usage hint makes dynamic buffers not a special case
anymore, simplifying things a bit.

intern/gawain/gawain/gwn_vertex_buffer.h

@@ -22,33 +22,41 @@
 
 // Is Gwn_VertBuf always used as part of a Gwn_Batch?
 
+typedef enum {
+	// can be extended to support more types
+	GWN_USAGE_STREAM,
+	GWN_USAGE_STATIC,
+	GWN_USAGE_DYNAMIC
+} Gwn_UsageType;
+
 typedef struct Gwn_VertBuf {
 	Gwn_VertFormat format;
 	unsigned vertex_ct;
-	unsigned alloc_ct; // size in vertex of alloced data
-#if VRAM_USAGE
-	unsigned vram_size; // size in byte of data present in the VRAM
-#endif
-	unsigned data_dirty : 1; // does the data has been touched since last transfert
-	unsigned data_dynamic : 1; // do we keep the RAM allocation for further updates?
-	unsigned data_resized : 1; // does the data has been resized since last transfert
-	GLubyte* data; // NULL indicates data in VRAM (unmapped) or not yet allocated
-	GLuint vbo_id; // 0 indicates not yet sent to VRAM
+	GLubyte* data; // NULL indicates data in VRAM (unmapped)
+	GLuint vbo_id; // 0 indicates not yet allocated
+	Gwn_UsageType usage; // usage hint for GL optimisation
 } Gwn_VertBuf;
 
-Gwn_VertBuf* GWN_vertbuf_create(void);
-Gwn_VertBuf* GWN_vertbuf_create_with_format(const Gwn_VertFormat*);
-Gwn_VertBuf* GWN_vertbuf_create_dynamic_with_format(const Gwn_VertFormat*);
+Gwn_VertBuf* GWN_vertbuf_create(Gwn_UsageType);
+Gwn_VertBuf* GWN_vertbuf_create_with_format_ex(const Gwn_VertFormat*, Gwn_UsageType);
+
+#define GWN_vertbuf_create_with_format(format) \
+	GWN_vertbuf_create_with_format_ex(format, GWN_USAGE_STATIC)
 
-void GWN_vertbuf_clear(Gwn_VertBuf*);
 void GWN_vertbuf_discard(Gwn_VertBuf*);
 
-void GWN_vertbuf_init(Gwn_VertBuf*);
-void GWN_vertbuf_init_with_format(Gwn_VertBuf*, const Gwn_VertFormat*);
+void GWN_vertbuf_init(Gwn_VertBuf*, Gwn_UsageType);
+void GWN_vertbuf_init_with_format_ex(Gwn_VertBuf*, const Gwn_VertFormat*, Gwn_UsageType);
+
+#define GWN_vertbuf_init_with_format(verts, format) \
+	GWN_vertbuf_init_with_format_ex(verts, format, GWN_USAGE_STATIC)
 
 unsigned GWN_vertbuf_size_get(const Gwn_VertBuf*);
 void GWN_vertbuf_data_alloc(Gwn_VertBuf*, unsigned v_ct);
-void GWN_vertbuf_data_resize(Gwn_VertBuf*, unsigned v_ct);
+void GWN_vertbuf_data_resize_ex(Gwn_VertBuf*, unsigned v_ct, bool keep_data);
+
+#define GWN_vertbuf_data_resize(verts, v_ct) \
+	GWN_vertbuf_data_resize_ex(verts, v_ct, true)
 
 // The most important set_attrib variant is the untyped one. Get it right first.
 // It takes a void* so the app developer is responsible for matching their app data types

intern/gawain/src/gwn_vertex_buffer.c

@@ -19,16 +19,26 @@
 
 static unsigned vbo_memory_usage;
 
-Gwn_VertBuf* GWN_vertbuf_create(void)
+static GLenum convert_usage_type_to_gl(Gwn_UsageType type)
+	{
+	static const GLenum table[] = {
+		[GWN_USAGE_STREAM] = GL_STREAM_DRAW,
+		[GWN_USAGE_STATIC] = GL_STATIC_DRAW,
+		[GWN_USAGE_DYNAMIC] = GL_DYNAMIC_DRAW
+		};
+	return table[type];
+	}
+
+Gwn_VertBuf* GWN_vertbuf_create(Gwn_UsageType usage)
 	{
 	Gwn_VertBuf* verts = malloc(sizeof(Gwn_VertBuf));
-	GWN_vertbuf_init(verts);
+	GWN_vertbuf_init(verts, usage);
 	return verts;
 	}
 
-Gwn_VertBuf* GWN_vertbuf_create_with_format(const Gwn_VertFormat* format)
+Gwn_VertBuf* GWN_vertbuf_create_with_format_ex(const Gwn_VertFormat* format, Gwn_UsageType usage)
 	{
-	Gwn_VertBuf* verts = GWN_vertbuf_create();
+	Gwn_VertBuf* verts = GWN_vertbuf_create(usage);
 	GWN_vertformat_copy(&verts->format, format);
 	if (!format->packed)
 		VertexFormat_pack(&verts->format);
@@ -38,61 +48,30 @@ Gwn_VertBuf* GWN_vertbuf_create_with_format(const Gwn_VertFormat* format)
 	// TODO: implement those memory savings
 	}
 
-Gwn_VertBuf* GWN_vertbuf_create_dynamic_with_format(const Gwn_VertFormat* format)
-	{
-	Gwn_VertBuf* verts = GWN_vertbuf_create_with_format(format);
-	verts->data_dynamic = true;
-	return verts;
-	}
-
-void GWN_vertbuf_init(Gwn_VertBuf* verts)
+void GWN_vertbuf_init(Gwn_VertBuf* verts, Gwn_UsageType usage)
 	{
 	memset(verts, 0, sizeof(Gwn_VertBuf));
+	verts->usage = usage;
 	}
 
-void GWN_vertbuf_init_with_format(Gwn_VertBuf* verts, const Gwn_VertFormat* format)
+void GWN_vertbuf_init_with_format_ex(Gwn_VertBuf* verts, const Gwn_VertFormat* format, Gwn_UsageType usage)
 	{
-	GWN_vertbuf_init(verts);
+	GWN_vertbuf_init(verts, usage);
 	GWN_vertformat_copy(&verts->format, format);
 	if (!format->packed)
 		VertexFormat_pack(&verts->format);
 	}
 
-/**
- * Like #GWN_vertbuf_discard but doesn't free.
- */
-void GWN_vertbuf_clear(Gwn_VertBuf* verts)
-	{
-	if (verts->vbo_id) {
-		GWN_buf_id_free(verts->vbo_id);
-#if VRAM_USAGE
-		vbo_memory_usage -= verts->vram_size;
-#endif
-	}
-
-	if (verts->data)
-		{
-		free(verts->data);
-		verts->data = NULL;
-		}
-	}
-
 void GWN_vertbuf_discard(Gwn_VertBuf* verts)
 	{
 	if (verts->vbo_id)
 		{
 		GWN_buf_id_free(verts->vbo_id);
 #if VRAM_USAGE
-		vbo_memory_usage -= verts->vram_size;
+		vbo_memory_usage -= GWN_vertbuf_size_get(verts);
 #endif
 		}
 
-	if (verts->data)
-		{
-		free(verts->data);
-		}
-
-
 	free(verts);
 	}
 
@@ -109,25 +88,82 @@ void GWN_vertbuf_data_alloc(Gwn_VertBuf* verts, unsigned v_ct)
 
 	verts->vertex_ct = v_ct;
 
-	// Data initially lives in main memory. Will be transferred to VRAM when we "prime" it.
-	verts->data = malloc(GWN_vertbuf_size_get(verts));
+#if TRUST_NO_ONE
+	assert(verts->vbo_id == 0);
+#endif
+
+	unsigned buffer_sz = GWN_vertbuf_size_get(verts);
+#if VRAM_USAGE
+	vbo_memory_usage += buffer_sz;
+#endif
+
+	// create an array buffer and map it to memory
+	verts->vbo_id = GWN_buf_id_alloc();
+	glBindBuffer(GL_ARRAY_BUFFER, verts->vbo_id);
+	glBufferData(GL_ARRAY_BUFFER, buffer_sz, NULL, convert_usage_type_to_gl(verts->usage));
+	verts->data = glMapBuffer(GL_ARRAY_BUFFER, GL_WRITE_ONLY);
 	}
 
-void GWN_vertbuf_data_resize(Gwn_VertBuf* verts, unsigned v_ct)
+void GWN_vertbuf_data_resize_ex(Gwn_VertBuf* verts, unsigned v_ct, bool keep_data)
 	{
 #if TRUST_NO_ONE
-	assert(verts->vertex_ct != v_ct); // allow this?
-	assert(verts->data != NULL); // has already been allocated
-	assert(verts->vbo_id == 0 || verts->data_dynamic); // has not been sent to VRAM
+	assert(verts->vbo_id != 0);
 #endif
 
-	// for dynamic buffers
-	verts->data_resized = true;
+	if (verts->vertex_ct == v_ct)
+		return;
 
+	unsigned old_buf_sz = GWN_vertbuf_size_get(verts);
 	verts->vertex_ct = v_ct;
-	verts->data = realloc(verts->data, GWN_vertbuf_size_get(verts));
-	// TODO: skip realloc if v_ct < existing vertex count
-	// extra space will be reclaimed, and never sent to VRAM (see VertexBuffer_prime)
+	unsigned new_buf_sz = GWN_vertbuf_size_get(verts);
+#if VRAM_USAGE
+	vbo_memory_usage += new_buf_sz - old_buf_sz;
+#endif
+
+	if (keep_data)
+		{
+		// we need to do a copy to keep the existing data
+		GLuint vbo_tmp;
+		glGenBuffers(1, &vbo_tmp);
+		// only copy the data that can fit in the new buffer
+		unsigned copy_sz = (old_buf_sz < new_buf_sz) ? old_buf_sz : new_buf_sz;
+		glBindBuffer(GL_COPY_WRITE_BUFFER, vbo_tmp);
+		glBufferData(GL_COPY_WRITE_BUFFER, copy_sz, NULL, GL_STREAM_COPY);
+
+		glBindBuffer(GL_COPY_READ_BUFFER, verts->vbo_id);
+		// we cannot copy from/to a mapped buffer
+		if (verts->data)
+			glUnmapBuffer(GL_COPY_READ_BUFFER);
+
+		// save data, resize the buffer, restore data
+		glCopyBufferSubData(GL_COPY_READ_BUFFER, GL_COPY_WRITE_BUFFER, 0, 0, copy_sz);
+		glBufferData(GL_COPY_READ_BUFFER, new_buf_sz, NULL, convert_usage_type_to_gl(verts->usage));
+		glCopyBufferSubData(GL_COPY_WRITE_BUFFER, GL_COPY_READ_BUFFER, 0, 0, copy_sz);
+
+		glDeleteBuffers(1, &vbo_tmp);
+		}
+	else
+		{
+		glBindBuffer(GL_COPY_READ_BUFFER, verts->vbo_id);
+		glBufferData(GL_COPY_READ_BUFFER, new_buf_sz, NULL, convert_usage_type_to_gl(verts->usage));
+		}
+
+	// if the buffer was mapped, update it's pointer
+	if (verts->data)
+		verts->data = glMapBuffer(GL_COPY_READ_BUFFER, GL_WRITE_ONLY);
+	}
+
+static void VertexBuffer_map(Gwn_VertBuf* verts)
+	{
+	glBindBuffer(GL_ARRAY_BUFFER, verts->vbo_id);
+	verts->data = glMapBuffer(GL_ARRAY_BUFFER, GL_WRITE_ONLY);
+	}
+
+static void VertexBuffer_unmap(Gwn_VertBuf* verts)
+	{
+	glBindBuffer(GL_ARRAY_BUFFER, verts->vbo_id);
+	glUnmapBuffer(GL_ARRAY_BUFFER);
+	verts->data = NULL;
 	}
 
 void GWN_vertbuf_attr_set(Gwn_VertBuf* verts, unsigned a_idx, unsigned v_idx, const void* data)
@@ -135,14 +171,14 @@ void GWN_vertbuf_attr_set(Gwn_VertBuf* verts, unsigned a_idx, unsigned v_idx, co
 	const Gwn_VertFormat* format = &verts->format;
 	const Gwn_VertAttr* a = format->attribs + a_idx;
 
-	verts->data_dirty = true;
-
 #if TRUST_NO_ONE
 	assert(a_idx < format->attrib_ct);
 	assert(v_idx < verts->vertex_ct);
-	assert(verts->data != NULL); // data must be in main mem
 #endif
 
+	if (verts->data == NULL)
+		VertexBuffer_map(verts);
+
 	memcpy((GLubyte*)verts->data + a->offset + v_idx * format->stride, data, a->sz);
 	}
 
@@ -151,8 +187,6 @@ void GWN_vertbuf_attr_fill(Gwn_VertBuf* verts, unsigned a_idx, const void* data)
 	const Gwn_VertFormat* format = &verts->format;
 	const Gwn_VertAttr* a = format->attribs + a_idx;
 
-	verts->data_dirty = true;
-
 #if TRUST_NO_ONE
 	assert(a_idx < format->attrib_ct);
 #endif
@@ -167,15 +201,15 @@ void GWN_vertbuf_attr_fill_stride(Gwn_VertBuf* verts, unsigned a_idx, unsigned s
 	const Gwn_VertFormat* format = &verts->format;
 	const Gwn_VertAttr* a = format->attribs + a_idx;
 
-	verts->data_dirty = true;
-
 #if TRUST_NO_ONE
 	assert(a_idx < format->attrib_ct);
-	assert(verts->data != NULL); // data must be in main mem
 #endif
 
 	const unsigned vertex_ct = verts->vertex_ct;
 
+	if (verts->data == NULL)
+		VertexBuffer_map(verts);
+
 	if (format->attrib_ct == 1 && stride == format->stride)
 		{
 		// we can copy it all at once
@@ -196,9 +230,11 @@ void GWN_vertbuf_attr_get_raw_data(Gwn_VertBuf* verts, unsigned a_idx, Gwn_VertB
 
 #if TRUST_NO_ONE
 	assert(a_idx < format->attrib_ct);
-	assert(verts->data != NULL); // data must be in main mem
 #endif
 
+	if (verts->data == NULL)
+		VertexBuffer_map(verts);
+
 	access->size = a->sz;
 	access->stride = format->stride;
 	access->data = (GLubyte*)verts->data + a->offset;
@@ -208,59 +244,11 @@ void GWN_vertbuf_attr_get_raw_data(Gwn_VertBuf* verts, unsigned a_idx, Gwn_VertB
 #endif
 	}
 
-
-static void VertexBuffer_prime(Gwn_VertBuf* verts)
-	{
-	unsigned buffer_sz = GWN_vertbuf_size_get(verts);
-
-#if VRAM_USAGE
-	vbo_memory_usage += buffer_sz;
-	verts->vram_size = buffer_sz;
-#endif
-
-	verts->vbo_id = GWN_buf_id_alloc();
-	glBindBuffer(GL_ARRAY_BUFFER, verts->vbo_id);
-	// fill with delicious data & send to GPU the first time only
-	glBufferData(GL_ARRAY_BUFFER, buffer_sz, verts->data, (verts->data_dynamic) ? GL_DYNAMIC_DRAW : GL_STATIC_DRAW);
-
-	// now that GL has a copy, discard original
-	if (!verts->data_dynamic)
-		{
-		free(verts->data);
-		verts->data = NULL;
-		}
-
-	verts->data_dirty = false;
-	}
-
-static void VertexBuffer_update(Gwn_VertBuf* verts)
-	{
-	unsigned buffer_sz = GWN_vertbuf_size_get(verts);
-
-#if VRAM_USAGE
-	vbo_memory_usage -= verts->vram_size;
-	vbo_memory_usage += buffer_sz;
-	verts->vram_size = buffer_sz;
-#endif
-
-	glBindBuffer(GL_ARRAY_BUFFER, verts->vbo_id);
-
-	// fill with delicious data & send to GPU ... AGAIN
-	if (verts->data_resized)
-		glBufferData(GL_ARRAY_BUFFER, buffer_sz, verts->data, GL_DYNAMIC_DRAW);
-	else
-		glBufferSubData(GL_ARRAY_BUFFER, 0, buffer_sz, verts->data); // .. todo try glMapBuffer
-
-	verts->data_dirty = false;
-	verts->data_resized = false;
-	}
-
 void GWN_vertbuf_use(Gwn_VertBuf* verts)
 	{
-	if (!verts->vbo_id)
-		VertexBuffer_prime(verts);
-	else if (verts->data_dirty)
-		VertexBuffer_update(verts);
+	if (verts->data)
+		// this also calls glBindBuffer
+		VertexBuffer_unmap(verts);
 	else
 		glBindBuffer(GL_ARRAY_BUFFER, verts->vbo_id);
 	}

source/blender/draw/intern/draw_instance_data.c

@@ -156,7 +156,7 @@ void DRW_batching_buffer_request(
 	}
 	/* Create the batch. */
 	bbuf = chunk->bbufs + new_id;
-	bbuf->vert = *r_vert = GWN_vertbuf_create_dynamic_with_format(format);
+	bbuf->vert = *r_vert = GWN_vertbuf_create_with_format_ex(format, GWN_USAGE_DYNAMIC);
 	bbuf->batch = *r_batch = GWN_batch_create_ex(type, bbuf->vert, NULL, 0);
 	bbuf->format = format;
 	bbuf->shgroup = shgroup;
@@ -197,7 +197,7 @@ void DRW_instancing_buffer_request(
 	}
 	/* Create the batch. */
 	ibuf = chunk->ibufs + new_id;
-	ibuf->vert = *r_vert = GWN_vertbuf_create_dynamic_with_format(format);
+	ibuf->vert = *r_vert = GWN_vertbuf_create_with_format_ex(format, GWN_USAGE_DYNAMIC);
 	ibuf->batch = *r_batch = GWN_batch_duplicate(instance);
 	ibuf->format = format;
 	ibuf->shgroup = shgroup;