[Bf-blender-cvs] SVN commit: /data/svn/bf-blender [58277] branches/soc-2013-depsgraph_mt/ source/blender/gpu/intern/gpu_buffers.c: Make GPU buffers allocation/ freeing safe for threading

[Sergey Sharybin]

Revision: 58277
          http://projects.blender.org/scm/viewvc.php?view=rev&root=bf-blender&revision=58277
Author:   nazgul
Date:     2013-07-15 18:46:08 +0000 (Mon, 15 Jul 2013)
Log Message:
-----------
Make GPU buffers allocation/freeing safe for threading

Code in GPU_buffers_free was already trying to be safe
for threading, by skipping OGL calls there, but in fact
it was still buggy.

Namely, freeing was doing buffers shift in a cycle, and
if two threads will call this function shifting will go
crazy.

Now made it so GPU_buffers_alloc and GPU_buffers_free
are using mutex lock, so they're completely safe for
threading. Same goes to gpu_buffer_setup function.

It required minor functions reshuffle, so there're no
locks happening from locked thread, but it's all very
straightforward change.

Modified Paths:
--------------
    branches/soc-2013-depsgraph_mt/source/blender/gpu/intern/gpu_buffers.c

Modified: branches/soc-2013-depsgraph_mt/source/blender/gpu/intern/gpu_buffers.c
===================================================================
--- branches/soc-2013-depsgraph_mt/source/blender/gpu/intern/gpu_buffers.c	2013-07-15 18:39:18 UTC (rev 58276)
+++ branches/soc-2013-depsgraph_mt/source/blender/gpu/intern/gpu_buffers.c	2013-07-15 18:46:08 UTC (rev 58277)
@@ -77,6 +77,8 @@
 static GPUBufferState GLStates = 0;
 static GPUAttrib attribData[MAX_GPU_ATTRIB_DATA] = { { -1, 0, 0 } };
 
+static ThreadMutex buffer_mutex = BLI_MUTEX_INITIALIZER;
+
 /* stores recently-deleted buffers so that new buffers won't have to
  * be recreated as often
  *
@@ -189,8 +191,11 @@
 }
 
 /* get a GPUBuffer of at least `size' bytes; uses one from the buffer
- * pool if possible, otherwise creates a new one */
-GPUBuffer *GPU_buffer_alloc(int size)
+ * pool if possible, otherwise creates a new one
+ *
+ * Thread-unsafe version for internal usage only.
+ */
+static GPUBuffer *gpu_buffer_alloc_intern(int size)
 {
 	GPUBufferPool *pool;
 	GPUBuffer *buf;
@@ -270,10 +275,30 @@
 	return buf;
 }
 
+/* Same as above, but safe for threading. */
+GPUBuffer *GPU_buffer_alloc(int size)
+{
+	GPUBuffer *buffer;
+
+	if (size == 0) {
+		/* Early out, no lock needed in this case. */
+		return NULL;
+	}
+
+	BLI_mutex_lock(&buffer_mutex);
+	buffer = gpu_buffer_alloc_intern(size);
+	BLI_mutex_unlock(&buffer_mutex);
+
+	return buffer;
+}
+
 /* release a GPUBuffer; does not free the actual buffer or its data,
  * but rather moves it to the pool of recently-freed buffers for
- * possible re-use*/
-void GPU_buffer_free(GPUBuffer *buffer)
+ * possible re-use
+ *
+ * Thread-unsafe version for internal usage only.
+ */
+static void gpu_buffer_free_intern(GPUBuffer *buffer)
 {
 	GPUBufferPool *pool;
 	int i;
@@ -312,6 +337,19 @@
 	pool->totbuf++;
 }
 
+/* Same as above, but safe for threading. */
+void GPU_buffer_free(GPUBuffer *buffer)
+{
+	if (!buffer) {
+		/* Early output, no need to lock in this case, */
+		return;
+	}
+
+	BLI_mutex_lock(&buffer_mutex);
+	gpu_buffer_free_intern(buffer);
+	BLI_mutex_unlock(&buffer_mutex);
+}
+
 typedef struct GPUVertPointLink {
 	struct GPUVertPointLink *next;
 	/* -1 means uninitialized */
@@ -496,13 +534,17 @@
 
 	pool = gpu_get_global_buffer_pool();
 
+	BLI_mutex_lock(&buffer_mutex);
+
 	/* alloc a GPUBuffer; fall back to legacy mode on failure */
-	if (!(buffer = GPU_buffer_alloc(size)))
+	if (!(buffer = gpu_buffer_alloc_intern(size)))
 		dm->drawObject->legacy = 1;
 
 	/* nothing to do for legacy mode */
-	if (dm->drawObject->legacy)
+	if (dm->drawObject->legacy) {
+		BLI_mutex_unlock(&buffer_mutex);
 		return NULL;
+	}
 
 	cur_index_per_mat = MEM_mallocN(sizeof(int) * object->totmaterial,
 	                                "GPU_buffer_setup.cur_index_per_mat");
@@ -527,7 +569,7 @@
 			/* attempt to map the buffer */
 			if (!(varray = glMapBufferARB(target, GL_WRITE_ONLY_ARB))) {
 				/* failed to map the buffer; delete it */
-				GPU_buffer_free(buffer);
+				gpu_buffer_free_intern(buffer);
 				gpu_buffer_pool_delete_last(pool);
 				buffer = NULL;
 
@@ -535,7 +577,7 @@
 				 * and reallocating the buffer */
 				if (pool->totbuf > 0) {
 					gpu_buffer_pool_delete_last(pool);
-					buffer = GPU_buffer_alloc(size);
+					buffer = gpu_buffer_alloc_intern(size);
 				}
 
 				/* allocation still failed; fall back
@@ -577,6 +619,8 @@
 
 	MEM_freeN(cur_index_per_mat);
 
+	BLI_mutex_unlock(&buffer_mutex);
+
 	return buffer;
 }