[Bf-blender-cvs] SVN commit: /data/svn/bf-blender [27541] trunk/blender/source/blender: Fixes for thread related render / compositing crashes:

Brecht Van Lommel brecht at blender.org
Tue Mar 16 17:58:45 CET 2010


Revision: 27541
          http://projects.blender.org/plugins/scmsvn/viewcvs.php?view=rev&root=bf-blender&revision=27541
Author:   blendix
Date:     2010-03-16 17:58:45 +0100 (Tue, 16 Mar 2010)

Log Message:
-----------
Fixes for thread related render / compositing crashes:

* Viewer node could free image while it is being redrawn, viewer image
  buffers now need acquire/release to be accessed as was already the
  case for render results.
* The Composite node could free the image buffers outside of a lock,
  also causing simultaneous redraw to crash.
* Especially on Windows, re-rendering could crash when drawing an image
  that was freed. When RE_RenderInProgress was true it would access the
  image buffer and simply return it while it could still contain a pointer
  to a render result buffer that was already freed. I don't understand
  why this case was there in the first place, so I've removed it.

Possibly fixes bugs #20174, #21418, #21391, #21394.

Modified Paths:
--------------
    trunk/blender/source/blender/blenkernel/intern/image.c
    trunk/blender/source/blender/blenlib/BLI_threads.h
    trunk/blender/source/blender/blenlib/intern/threads.c
    trunk/blender/source/blender/nodes/intern/CMP_nodes/CMP_composite.c
    trunk/blender/source/blender/nodes/intern/CMP_nodes/CMP_viewer.c
    trunk/blender/source/blender/render/intern/source/pipeline.c

Modified: trunk/blender/source/blender/blenkernel/intern/image.c
===================================================================
--- trunk/blender/source/blender/blenkernel/intern/image.c	2010-03-16 16:15:30 UTC (rev 27540)
+++ trunk/blender/source/blender/blenkernel/intern/image.c	2010-03-16 16:58:45 UTC (rev 27541)
@@ -1932,113 +1932,93 @@
 /* always returns a single ibuf, also during render progress */
 static ImBuf *image_get_render_result(Image *ima, ImageUser *iuser, void **lock_r)
 {
-	Render *re= NULL;
-	RenderResult *rr= NULL;
-	
+	Render *re;
+	RenderResult rres;
+	float *rectf, *rectz;
+	unsigned int *rect;
+	float dither;
+	int channels, layer, pass;
+	ImBuf *ibuf;
+
+	if(!(iuser && iuser->scene))
+		return 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, RE_SLOT_VIEW);
-		rr= RE_AcquireResultRead(re);
+	re= RE_GetRender(iuser->scene->id.name, RE_SLOT_VIEW);
 
-		/* release is done in BKE_image_release_ibuf using lock_r */
-		*lock_r= re;
-	}
-
-	if(rr==NULL)
-		return NULL;
+	channels= 4;
+	layer= (iuser)? iuser->layer: 0;
+	pass= (iuser)? iuser->pass: 0;
 	
-	if(RE_RenderInProgress(re)) {
-		ImBuf *ibuf= image_get_ibuf(ima, IMA_NO_INDEX, 0);
-		
-		/* make ibuf if needed, and initialize it */
-		/* this only gets called when mutex locked */
-		if(ibuf==NULL) {
-			ibuf= IMB_allocImBuf(rr->rectx, rr->recty, 32, IB_rect, 0);
-			image_assign_ibuf(ima, ibuf, IMA_NO_INDEX, 0);
-		}
+	/* this gives active layer, composite or seqence result */
+	RE_AcquireResultImage(re, &rres);
+	rect= (unsigned int *)rres.rect32;
+	rectf= rres.rectf;
+	rectz= rres.rectz;
+	dither= iuser->scene->r.dither_intensity;
 
-		return ibuf;
-	}
-	else {
-		RenderResult rres;
-		float *rectf, *rectz;
-		unsigned int *rect;
-		float dither;
-		int channels, layer, pass;
+	/* get compo/seq result by default */
+	if(rres.rectf && layer==0);
+	else if(rres.layers.first) {
+		RenderLayer *rl= BLI_findlink(&rres.layers, layer-(rres.rectf?1:0));
+		if(rl) {
+			RenderPass *rpass;
 
-		channels= 4;
-		layer= (iuser)? iuser->layer: 0;
-		pass= (iuser)? iuser->pass: 0;
-		
-		/* this gives active layer, composite or seqence result */
-		RE_AcquireResultImage(RE_GetRender(iuser->scene->id.name, RE_SLOT_VIEW), &rres);
-		rect= (unsigned int *)rres.rect32;
-		rectf= rres.rectf;
-		rectz= rres.rectz;
-		dither= iuser->scene->r.dither_intensity;
-
-		/* get compo/seq result by default */
-		if(rr->rectf && layer==0);
-		else if(rr->layers.first) {
-			RenderLayer *rl= BLI_findlink(&rr->layers, layer-(rr->rectf?1:0));
-			if(rl) {
-				RenderPass *rpass;
-
-				/* there's no combined pass, is in renderlayer itself */
-				if(pass==0) {
-					rectf= rl->rectf;
+			/* there's no combined pass, is in renderlayer itself */
+			if(pass==0) {
+				rectf= rl->rectf;
+			}
+			else {
+				rpass= BLI_findlink(&rl->passes, pass-1);
+				if(rpass) {
+					channels= rpass->channels;
+					rectf= rpass->rect;
+					dither= 0.0f; /* don't dither passes */
 				}
-				else {
-					rpass= BLI_findlink(&rl->passes, pass-1);
-					if(rpass) {
-						channels= rpass->channels;
-						rectf= rpass->rect;
-						dither= 0.0f; /* don't dither passes */
-					}
-				}
-
-				for(rpass= rl->passes.first; rpass; rpass= rpass->next)
-					if(rpass->passtype == SCE_PASS_Z)
-						rectz= rpass->rect;
 			}
-		}
-		
-		if(rectf || rect) {
-			ImBuf *ibuf= image_get_ibuf(ima, IMA_NO_INDEX, 0);
 
-			/* make ibuf if needed, and initialize it */
-			if(ibuf==NULL) {
-				ibuf= IMB_allocImBuf(rr->rectx, rr->recty, 32, 0, 0);
-				image_assign_ibuf(ima, ibuf, IMA_NO_INDEX, 0);
-			}
-			ibuf->x= rr->rectx;
-			ibuf->y= rr->recty;
-			
-			if(ibuf->rect_float!=rectf || rect) /* ensure correct redraw */
-				imb_freerectImBuf(ibuf);
-			if(rect)
-				ibuf->rect= rect;
-			
-			ibuf->rect_float= rectf;
-			ibuf->flags |= IB_rectfloat;
-			ibuf->channels= channels;
-			ibuf->zbuf_float= rectz;
-			ibuf->flags |= IB_zbuffloat;
-			ibuf->dither= dither;
-
-			RE_ReleaseResultImage(re);
-
-			ima->ok= IMA_OK_LOADED;
-			return ibuf;
+			for(rpass= rl->passes.first; rpass; rpass= rpass->next)
+				if(rpass->passtype == SCE_PASS_Z)
+					rectz= rpass->rect;
 		}
-
+	}
+	
+	if(!(rectf || rect)) {
 		RE_ReleaseResultImage(re);
+		return NULL;
 	}
+
+	ibuf= image_get_ibuf(ima, IMA_NO_INDEX, 0);
+
+	/* make ibuf if needed, and initialize it */
+	if(ibuf==NULL) {
+		ibuf= IMB_allocImBuf(rres.rectx, rres.recty, 32, 0, 0);
+		image_assign_ibuf(ima, ibuf, IMA_NO_INDEX, 0);
+	}
+	ibuf->x= rres.rectx;
+	ibuf->y= rres.recty;
 	
-	return NULL;
+	if(ibuf->rect_float!=rectf || rect) /* ensure correct redraw */
+		imb_freerectImBuf(ibuf);
+	if(rect)
+		ibuf->rect= rect;
+	
+	ibuf->rect_float= rectf;
+	ibuf->flags |= IB_rectfloat;
+	ibuf->channels= channels;
+	ibuf->zbuf_float= rectz;
+	ibuf->flags |= IB_zbuffloat;
+	ibuf->dither= dither;
+
+	ima->ok= IMA_OK_LOADED;
+
+	/* release is done in BKE_image_release_ibuf using lock_r */
+	*lock_r= re;
+
+	return ibuf;
 }
 
 static ImBuf *image_get_ibuf_threadsafe(Image *ima, ImageUser *iuser, int *frame_r, int *index_r)
@@ -2199,10 +2179,17 @@
 					ibuf= image_get_render_result(ima, iuser, lock_r);
 				}
 				else if(ima->type==IMA_TYPE_COMPOSITE) {
-					/* Composite Viewer, all handled in compositor */
-					/* fake ibuf, will be filled in compositor */
-					ibuf= IMB_allocImBuf(256, 256, 32, IB_rect, 0);
-					image_assign_ibuf(ima, ibuf, 0, frame);
+					/* requires lock/unlock, otherwise don't return image */
+					if(lock_r) {
+						/* unlock in BKE_image_release_ibuf */
+						BLI_lock_thread(LOCK_VIEWER);
+						*lock_r= ima;
+
+						/* Composite Viewer, all handled in compositor */
+						/* fake ibuf, will be filled in compositor */
+						ibuf= IMB_allocImBuf(256, 256, 32, IB_rect, 0);
+						image_assign_ibuf(ima, ibuf, 0, frame);
+					}
 				}
 			}
 		}
@@ -2220,9 +2207,11 @@
 
 void BKE_image_release_ibuf(Image *ima, void *lock)
 {
-	/* for getting image during threaded render, need to release */
-	if(lock)
-		RE_ReleaseResult(lock);
+	/* for getting image during threaded render / compositing, need to release */
+	if(lock == ima)
+		BLI_unlock_thread(LOCK_VIEWER); /* viewer image */
+	else if(lock)
+		RE_ReleaseResultImage(lock); /* render result */
 }
 
 ImBuf *BKE_image_get_ibuf(Image *ima, ImageUser *iuser)

Modified: trunk/blender/source/blender/blenlib/BLI_threads.h
===================================================================
--- trunk/blender/source/blender/blenlib/BLI_threads.h	2010-03-16 16:15:30 UTC (rev 27540)
+++ trunk/blender/source/blender/blenlib/BLI_threads.h	2010-03-16 16:58:45 UTC (rev 27541)
@@ -59,7 +59,8 @@
 
 #define LOCK_IMAGE		0
 #define LOCK_PREVIEW	1
-#define LOCK_CUSTOM1	2
+#define LOCK_VIEWER		2
+#define LOCK_CUSTOM1	3
 
 void	BLI_lock_thread(int type);
 void	BLI_unlock_thread(int type);

Modified: trunk/blender/source/blender/blenlib/intern/threads.c
===================================================================
--- trunk/blender/source/blender/blenlib/intern/threads.c	2010-03-16 16:15:30 UTC (rev 27540)
+++ trunk/blender/source/blender/blenlib/intern/threads.c	2010-03-16 16:58:45 UTC (rev 27541)
@@ -109,6 +109,7 @@
 static pthread_mutex_t _malloc_lock = PTHREAD_MUTEX_INITIALIZER;
 static pthread_mutex_t _image_lock = PTHREAD_MUTEX_INITIALIZER;
 static pthread_mutex_t _preview_lock = PTHREAD_MUTEX_INITIALIZER;
+static pthread_mutex_t _viewer_lock = PTHREAD_MUTEX_INITIALIZER;
 static pthread_mutex_t _custom1_lock = PTHREAD_MUTEX_INITIALIZER;
 static int thread_levels= 0;	/* threads can be invoked inside threads */
 
@@ -327,6 +328,8 @@
 		pthread_mutex_lock(&_image_lock);
 	else if (type==LOCK_PREVIEW)
 		pthread_mutex_lock(&_preview_lock);
+	else if (type==LOCK_VIEWER)
+		pthread_mutex_lock(&_viewer_lock);
 	else if (type==LOCK_CUSTOM1)
 		pthread_mutex_lock(&_custom1_lock);
 }
@@ -337,6 +340,8 @@
 		pthread_mutex_unlock(&_image_lock);
 	else if (type==LOCK_PREVIEW)
 		pthread_mutex_unlock(&_preview_lock);
+	else if (type==LOCK_VIEWER)
+		pthread_mutex_unlock(&_viewer_lock);
 	else if(type==LOCK_CUSTOM1)
 		pthread_mutex_unlock(&_custom1_lock);
 }

Modified: trunk/blender/source/blender/nodes/intern/CMP_nodes/CMP_composite.c
===================================================================
--- trunk/blender/source/blender/nodes/intern/CMP_nodes/CMP_composite.c	2010-03-16 16:15:30 UTC (rev 27540)
+++ trunk/blender/source/blender/nodes/intern/CMP_nodes/CMP_composite.c	2010-03-16 16:58:45 UTC (rev 27541)
@@ -80,10 +80,10 @@
 				outbuf->malloc= 0;
 				free_compbuf(outbuf);
 
-				RE_ReleaseResult(re);
-				
 				/* signal for imageviewer to refresh (it converts to byte rects...) */
 				BKE_image_signal(BKE_image_verify_viewer(IMA_TYPE_R_RESULT, "Render Result"), NULL, IMA_SIGNAL_FREE);
+
+				RE_ReleaseResult(re);
 				return;
 			}
 			else

Modified: trunk/blender/source/blender/nodes/intern/CMP_nodes/CMP_viewer.c
===================================================================
--- trunk/blender/source/blender/nodes/intern/CMP_nodes/CMP_viewer.c	2010-03-16 16:15:30 UTC (rev 27540)
+++ trunk/blender/source/blender/nodes/intern/CMP_nodes/CMP_viewer.c	2010-03-16 16:58:45 UTC (rev 27541)
@@ -50,13 +50,15 @@
 		ImBuf *ibuf;
 		CompBuf *cbuf, *tbuf;
 		int rectx, recty;
+		void *lock;
 		
 		BKE_image_user_calc_frame(node->storage, rd->cfra, 0);
 
 		/* always returns for viewer image, but we check nevertheless */
-		ibuf= BKE_image_get_ibuf(ima, node->storage);
+		ibuf= BKE_image_acquire_ibuf(ima, node->storage, &lock);
 		if(ibuf==NULL) {
 			printf("node_composit_exec_viewer error\n");
+			BKE_image_release_ibuf(ima, lock);
 			return;
 		}
 		
@@ -106,6 +108,8 @@
 			free_compbuf(zbuf);
 		}
 
+		BKE_image_release_ibuf(ima, lock);
+
 		generate_preview(data, node, cbuf);
 		free_compbuf(cbuf);
 

Modified: trunk/blender/source/blender/render/intern/source/pipeline.c
===================================================================
--- trunk/blender/source/blender/render/intern/source/pipeline.c	2010-03-16 16:15:30 UTC (rev 27540)
+++ trunk/blender/source/blender/render/intern/source/pipeline.c	2010-03-16 16:58:45 UTC (rev 27541)
@@ -138,11 +138,6 @@
 static void print_error(void *unused, char *str) {printf("ERROR: %s\n", str);}
 static int default_break(void *unused) {return G.afbreek == 1;}
 
-int RE_RenderInProgress(Render *re)
-{
-	return re->result_ok==0;
-}
-

@@ Diff output truncated at 10240 characters. @@




More information about the Bf-blender-cvs mailing list