[Bf-blender-cvs] SVN commit: /data/svn/bf-blender [23570] trunk/blender: Render & Compositing Thread Fixes

Brecht Van Lommel brecht at blender.org
Wed Sep 30 20:18:33 CEST 2009


Revision: 23570
          http://projects.blender.org/plugins/scmsvn/viewcvs.php?view=rev&root=bf-blender&revision=23570
Author:   blendix
Date:     2009-09-30 20:18:32 +0200 (Wed, 30 Sep 2009)

Log Message:
-----------
Render & Compositing Thread Fixes

* Rendering twice or more could crash layer/pass buttons.
* Compositing would crash while drawing the image.
* Rendering animations could also crash drawing the image.
* Compositing could crash 
* Starting to rendering while preview render / compo was
  still running could crash.
* Exiting while rendering an animation would not abort the
  renderer properly, making Blender seemingly freeze.
* Fixes theoretically possible issue with setting malloc
  lock with nested threads.
* Drawing previews inside nodes could crash when those nodes
  were being rendered at the same time.

There's more crashes, manipulating the scene data or undo can
still crash, this commit only focuses on making sure the image
buffer and render result access is thread safe.


Implementation:
* Rather than assuming the render result does not get freed
  during render, which seems to be quite difficult to do given
  that e.g. the compositor is allowed to change the size of
  the buffer or output different passes, the render result is
  now protected with a read/write mutex.
* The read/write mutex allows multiple readers (and pixel
  writers) at the same time, but only allows one writer to
  manipulate the data structure.
* Added BKE_image_acquire_ibuf/BKE_image_release_ibuf to access
  images being rendered, cases where this is not needed (most
  code) can still use BKE_image_get_ibuf.
* The job manager now allows only one rendering job at the same
  time, rather than the G.rendering check which was not reliable.

Modified Paths:
--------------
    trunk/blender/intern/guardedalloc/intern/mallocn.c
    trunk/blender/source/blender/blenkernel/BKE_image.h
    trunk/blender/source/blender/blenkernel/intern/image.c
    trunk/blender/source/blender/blenkernel/intern/node.c
    trunk/blender/source/blender/blenkernel/intern/sequence.c
    trunk/blender/source/blender/blenlib/BLI_threads.h
    trunk/blender/source/blender/blenlib/intern/threads.c
    trunk/blender/source/blender/editors/include/ED_image.h
    trunk/blender/source/blender/editors/render/render_preview.c
    trunk/blender/source/blender/editors/screen/screen_ops.c
    trunk/blender/source/blender/editors/screen/screendump.c
    trunk/blender/source/blender/editors/space_file/writeimage.c
    trunk/blender/source/blender/editors/space_image/image_buttons.c
    trunk/blender/source/blender/editors/space_image/image_draw.c
    trunk/blender/source/blender/editors/space_image/image_ops.c
    trunk/blender/source/blender/editors/space_image/space_image.c
    trunk/blender/source/blender/editors/space_node/node_draw.c
    trunk/blender/source/blender/editors/space_node/node_edit.c
    trunk/blender/source/blender/makesrna/intern/rna_image.c
    trunk/blender/source/blender/makesrna/intern/rna_space.c
    trunk/blender/source/blender/nodes/intern/CMP_nodes/CMP_composite.c
    trunk/blender/source/blender/nodes/intern/CMP_nodes/CMP_image.c
    trunk/blender/source/blender/nodes/intern/CMP_util.c
    trunk/blender/source/blender/render/extern/include/RE_pipeline.h
    trunk/blender/source/blender/render/intern/include/render_types.h
    trunk/blender/source/blender/render/intern/source/pipeline.c
    trunk/blender/source/blender/render/intern/source/sss.c
    trunk/blender/source/blender/windowmanager/WM_api.h
    trunk/blender/source/blender/windowmanager/intern/wm_jobs.c

Modified: trunk/blender/intern/guardedalloc/intern/mallocn.c
===================================================================
--- trunk/blender/intern/guardedalloc/intern/mallocn.c	2009-09-30 17:13:57 UTC (rev 23569)
+++ trunk/blender/intern/guardedalloc/intern/mallocn.c	2009-09-30 18:18:32 UTC (rev 23570)
@@ -688,17 +688,35 @@
 
 uintptr_t MEM_get_memory_in_use(void)
 {
-	return mem_in_use;
+	uintptr_t _mem_in_use;
+
+	mem_lock_thread();
+	_mem_in_use= mem_in_use;
+	mem_unlock_thread();
+
+	return _mem_in_use;
 }
 
 uintptr_t MEM_get_mapped_memory_in_use(void)
 {
-	return mmap_in_use;
+	uintptr_t _mmap_in_use;
+
+	mem_lock_thread();
+	_mmap_in_use= mmap_in_use;
+	mem_unlock_thread();
+
+	return _mmap_in_use;
 }
 
 int MEM_get_memory_blocks_in_use(void)
 {
-	return totblock;
+	int _totblock;
+
+	mem_lock_thread();
+	_totblock= totblock;
+	mem_unlock_thread();
+
+	return _totblock;
 }
 
 /* eof */

Modified: trunk/blender/source/blender/blenkernel/BKE_image.h
===================================================================
--- trunk/blender/source/blender/blenkernel/BKE_image.h	2009-09-30 17:13:57 UTC (rev 23569)
+++ trunk/blender/source/blender/blenkernel/BKE_image.h	2009-09-30 18:18:32 UTC (rev 23570)
@@ -107,6 +107,11 @@
 /* always call to make signals work */
 struct ImBuf *BKE_image_get_ibuf(struct Image *ima, struct ImageUser *iuser);
 
+/* same as above, but can be used to retrieve images being rendered in
+ * a thread safe way, always call both acquire and release */
+struct ImBuf *BKE_image_acquire_ibuf(struct Image *ima, struct ImageUser *iuser, void **lock_r);
+void BKE_image_release_ibuf(struct Image *ima, void *lock);
+
 /* returns existing Image when filename/type is same (frame optional) */
 struct Image *BKE_add_image_file(const char *name, int frame);
 
@@ -135,7 +140,8 @@
 struct RenderPass *BKE_image_multilayer_index(struct RenderResult *rr, struct ImageUser *iuser);
 
 /* for multilayer images as well as for render-viewer */
-struct RenderResult *BKE_image_get_renderresult(struct Scene *scene, struct Image *ima);
+struct RenderResult *BKE_image_acquire_renderresult(struct Scene *scene, struct Image *ima);
+void BKE_image_release_renderresult(struct Scene *scene, struct Image *ima);
 
 /* goes over all textures that use images */
 void	BKE_image_free_all_textures(void);

Modified: trunk/blender/source/blender/blenkernel/intern/image.c
===================================================================
--- trunk/blender/source/blender/blenkernel/intern/image.c	2009-09-30 17:13:57 UTC (rev 23569)
+++ trunk/blender/source/blender/blenkernel/intern/image.c	2009-09-30 18:18:32 UTC (rev 23570)
@@ -1564,15 +1564,22 @@
 	return rpass;
 }
 
-RenderResult *BKE_image_get_renderresult(struct Scene *scene, Image *ima)
+RenderResult *BKE_image_acquire_renderresult(struct Scene *scene, Image *ima)
 {
 	if(ima->rr)
 		return ima->rr;
-	if(ima->type==IMA_TYPE_R_RESULT)
-		return RE_GetResult(RE_GetRender(scene->id.name));
+	else if(ima->type==IMA_TYPE_R_RESULT)
+		return RE_AcquireResultRead(RE_GetRender(scene->id.name));
 	return NULL;
 }
 
+void BKE_image_release_renderresult(struct Scene *scene, Image *ima)
+{
+	if(ima->rr);
+	else if(ima->type==IMA_TYPE_R_RESULT)
+		RE_ReleaseResult(RE_GetRender(scene->id.name));
+}
+
 /* after imbuf load, openexr type can return with a exrhandle open */
 /* in that case we have to build a render-result */
 static void image_create_multilayer(Image *ima, ImBuf *ibuf, int framenr)
@@ -1873,16 +1880,25 @@
 /* showing RGBA result itself (from compo/sequence) or
    like exr, using layers etc */
 /* always returns a single ibuf, also during render progress */
-static ImBuf *image_get_render_result(Image *ima, ImageUser *iuser)
+static ImBuf *image_get_render_result(Image *ima, ImageUser *iuser, void **lock_r)
 {
 	Render *re= NULL;
 	RenderResult *rr= NULL;
 	
+	/* if we the caller is not going to release the lock, don't give the image */
+	if(!lock_r)
+		return NULL;
+
 	if(iuser && iuser->scene) {
 		re= RE_GetRender(iuser->scene->id.name);
-		rr= RE_GetResult(re);
+		rr= RE_AcquireResultRead(re);
+
+		/* release is done in BKE_image_release_ibuf using lock_r */
+		*lock_r= re;
 	}
-	if(rr==NULL) return NULL;
+
+	if(rr==NULL)
+		return NULL;
 	
 	if(RE_RenderInProgress(re)) {
 		ImBuf *ibuf= image_get_ibuf(ima, IMA_NO_INDEX, 0);
@@ -1893,6 +1909,7 @@
 			ibuf= IMB_allocImBuf(rr->rectx, rr->recty, 32, IB_rect, 0);
 			image_assign_ibuf(ima, ibuf, IMA_NO_INDEX, 0);
 		}
+
 		return ibuf;
 	}
 	else {
@@ -1907,7 +1924,7 @@
 		pass= (iuser)? iuser->pass: 0;
 		
 		/* this gives active layer, composite or seqence result */
-		RE_GetResultImage(RE_GetRender(iuser->scene->id.name), &rres);
+		RE_AcquireResultImage(RE_GetRender(iuser->scene->id.name), &rres);
 		rect= (unsigned int *)rres.rect32;
 		rectf= rres.rectf;
 		dither= iuser->scene->r.dither_intensity;
@@ -1954,10 +1971,14 @@
 			ibuf->zbuf_float= rres.rectz;
 			ibuf->flags |= IB_zbuffloat;
 			ibuf->dither= dither;
-			
+
+			RE_ReleaseResultImage(re);
+
 			ima->ok= IMA_OK_LOADED;
 			return ibuf;
 		}
+
+		RE_ReleaseResultImage(re);
 	}
 	
 	return NULL;
@@ -2011,8 +2032,9 @@
 }
 
 /* Checks optional ImageUser and verifies/creates ImBuf. */
-/* returns ibuf */
-ImBuf *BKE_image_get_ibuf(Image *ima, ImageUser *iuser)
+/* use this one if you want to get a render result in progress,
+ * if not, use BKE_image_get_ibuf which doesn't require a release */
+ImBuf *BKE_image_acquire_ibuf(Image *ima, ImageUser *iuser, void **lock_r)
 {
 	ImBuf *ibuf= NULL;
 	float color[] = {0, 0, 0, 1};
@@ -2028,6 +2050,9 @@
 	 * things in a threadsafe way for image_get_ibuf_threadsafe to work correct.
 	 * That means, the last two steps must be, 1) add the ibuf to the list and
 	 * 2) set ima/iuser->ok to 0 to IMA_OK_LOADED */
+	
+	if(lock_r)
+		*lock_r= NULL;
 
 	/* quick reject tests */
 	if(ima==NULL) 
@@ -2103,8 +2128,9 @@
 			}
 			else if(ima->source == IMA_SRC_VIEWER) {
 				if(ima->type==IMA_TYPE_R_RESULT) {
-					/* always verify entirely */
-					ibuf= image_get_render_result(ima, iuser);
+					/* always verify entirely, and potentially
+					   returns pointer to release later */
+					ibuf= image_get_render_result(ima, iuser, lock_r);
 				}
 				else if(ima->type==IMA_TYPE_COMPOSITE) {
 					/* Composite Viewer, all handled in compositor */
@@ -2126,7 +2152,18 @@
 	return ibuf;
 }
 
+void BKE_image_release_ibuf(Image *ima, void *lock)
+{
+	/* for getting image during threaded render, need to release */
+	if(lock)
+		RE_ReleaseResult(lock);
+}
 
+ImBuf *BKE_image_get_ibuf(Image *ima, ImageUser *iuser)
+{
+	return BKE_image_acquire_ibuf(ima, iuser, NULL);
+}
+
 void BKE_image_user_calc_imanr(ImageUser *iuser, int cfra, int fieldnr)
 {
 	int imanr, len;

Modified: trunk/blender/source/blender/blenkernel/intern/node.c
===================================================================
--- trunk/blender/source/blender/blenkernel/intern/node.c	2009-09-30 17:13:57 UTC (rev 23569)
+++ trunk/blender/source/blender/blenkernel/intern/node.c	2009-09-30 18:18:32 UTC (rev 23570)
@@ -1953,6 +1953,7 @@
 			if(ns->data) {
 				printf("freed leftover buffer from stack\n");
 				free_compbuf(ns->data);
+				ns->data= NULL;
 			}
 		}
 	}

Modified: trunk/blender/source/blender/blenkernel/intern/sequence.c
===================================================================
--- trunk/blender/source/blender/blenkernel/intern/sequence.c	2009-09-30 17:13:57 UTC (rev 23569)
+++ trunk/blender/source/blender/blenkernel/intern/sequence.c	2009-09-30 18:18:32 UTC (rev 23570)
@@ -2067,7 +2067,7 @@
 			if(rendering)
 				BLI_strncpy(sce->id.name+2, scenename, 64);
 			
-			RE_GetResultImage(re, &rres);
+			RE_AcquireResultImage(re, &rres);
 			
 			if(rres.rectf) {
 				se->ibuf= IMB_allocImBuf(rres.rectx, rres.recty, 32, IB_rectfloat, 0);
@@ -2080,6 +2080,8 @@
 				se->ibuf= IMB_allocImBuf(rres.rectx, rres.recty, 32, IB_rect, 0);
 				memcpy(se->ibuf->rect, rres.rect32, 4*rres.rectx*rres.recty);
 			}
+
+			RE_ReleaseResultImage(re);
 			
 			BIF_end_render_callbacks();
 			

Modified: trunk/blender/source/blender/blenlib/BLI_threads.h
===================================================================
--- trunk/blender/source/blender/blenlib/BLI_threads.h	2009-09-30 17:13:57 UTC (rev 23569)
+++ trunk/blender/source/blender/blenlib/BLI_threads.h	2009-09-30 18:18:32 UTC (rev 23570)
@@ -31,14 +31,15 @@
 #ifndef BLI_THREADS_H
 #define BLI_THREADS_H 
 
-/* one custom lock available now. can be extended */
-#define LOCK_IMAGE		0
-#define LOCK_CUSTOM1	1
+#include <pthread.h>
 
 /* for tables, button in UI, etc */
 #define BLENDER_MAX_THREADS		8
 
 struct ListBase;
+
+/* Threading API */
+
 void	BLI_init_threads	(struct ListBase *threadbase, void *(*do_thread)(void *), int tot);
 int		BLI_available_threads(struct ListBase *threadbase);
 int		BLI_available_thread_index(struct ListBase *threadbase);
@@ -48,19 +49,47 @@
 void	BLI_remove_threads(struct ListBase *threadbase);
 void	BLI_end_threads		(struct ListBase *threadbase);
 
-void	BLI_lock_thread		(int type);
-void	BLI_unlock_thread	(int type);
+/* System Information */
 
-int		BLI_system_thread_count( void ); /* gets the number of threads the system can make use of */
+int		BLI_system_thread_count(void); /* gets the number of threads the system can make use of */
 
-		/* exported by preview render, it has to ensure render buffers are not freed while draw */
-void	BLI_lock_malloc_thread(void);
-void	BLI_unlock_malloc_thread(void);
+/* Global Mutex Locks
+ * 
+ * One custom lock available now. can be extended. */
 
-/* ThreadedWorker is a simple tool for dispatching work to a limited number of threads in a transparent
- * fashion from the caller's perspective
- * */
+#define LOCK_IMAGE		0
+#define LOCK_PREVIEW	1
+#define LOCK_CUSTOM1	2
 
+void	BLI_lock_thread(int type);
+void	BLI_unlock_thread(int type);
+
+/* Mutex Lock */
+
+typedef pthread_mutex_t ThreadMutex;
+
+void BLI_mutex_init(ThreadMutex *mutex);
+void BLI_mutex_lock(ThreadMutex *mutex);
+void BLI_mutex_unlock(ThreadMutex *mutex);
+void BLI_mutex_end(ThreadMutex *mutex);
+
+/* Read/Write Mutex Lock */
+
+#define THREAD_LOCK_READ	1
+#define THREAD_LOCK_WRITE	2
+
+typedef pthread_rwlock_t ThreadRWMutex;
+
+void BLI_rw_mutex_init(ThreadRWMutex *mutex);
+void BLI_rw_mutex_lock(ThreadRWMutex *mutex, int mode);
+void BLI_rw_mutex_unlock(ThreadRWMutex *mutex);
+void BLI_rw_mutex_end(ThreadRWMutex *mutex);
+
+/* ThreadedWorker
+ *
+ * A simple tool for dispatching work to a limited number of threads

@@ Diff output truncated at 10240 characters. @@




More information about the Bf-blender-cvs mailing list