[Bf-blender-cvs] SVN commit: /data/svn/bf-blender [48893] trunk/blender/source/blender: Fix #32087: Crash while changing values in comp editor ( bt and blender included)

Sergey Sharybin sergey.vfx at gmail.com
Fri Jul 13 15:47:14 CEST 2012


Revision: 48893
          http://projects.blender.org/scm/viewvc.php?view=rev&root=bf-blender&revision=48893
Author:   nazgul
Date:     2012-07-13 13:47:13 +0000 (Fri, 13 Jul 2012)
Log Message:
-----------
Fix #32087: Crash while changing values in comp editor (bt and blender included)

Issue was caused by threading conflict between compositor output node which
is freeing buffers used by render result image and image draw code which
could use buffers at the same time as compositor frees this buffers.

Solved by adding adding  lock around viewer image invalidation and image
drawing.

Use renamed LOCK_PREVIEW mutex for this, which si not called LOCK_DRAW_IMAGE.
With new compositor locking for preview is not needed so it could be removed.

Added the same lock around viewer operation which also frees buffers used
by viewer image. It's actually quite difficult to check whether this is
indeed needed. This code seems to be using acquire/release technique, but
somehow acquiring ImBuf before invalidating it in compositor operation
doesn't resolve the issue, so probably it's not actually locking acquire
and things should be checked deeper.

Modified Paths:
--------------
    trunk/blender/source/blender/blenlib/BLI_threads.h
    trunk/blender/source/blender/blenlib/intern/threads.c
    trunk/blender/source/blender/compositor/operations/COM_CompositorOperation.cpp
    trunk/blender/source/blender/compositor/operations/COM_ViewerBaseOperation.cpp
    trunk/blender/source/blender/editors/space_image/image_draw.c
    trunk/blender/source/blender/editors/space_node/node_draw.c
    trunk/blender/source/blender/nodes/composite/node_composite_util.c

Modified: trunk/blender/source/blender/blenlib/BLI_threads.h
===================================================================
--- trunk/blender/source/blender/blenlib/BLI_threads.h	2012-07-13 12:55:30 UTC (rev 48892)
+++ trunk/blender/source/blender/blenlib/BLI_threads.h	2012-07-13 13:47:13 UTC (rev 48893)
@@ -70,7 +70,7 @@
  * One custom lock available now. can be extended. */
 
 #define LOCK_IMAGE      0
-#define LOCK_PREVIEW    1
+#define LOCK_DRAW_IMAGE 1
 #define LOCK_VIEWER     2
 #define LOCK_CUSTOM1    3
 #define LOCK_RCACHE     4

Modified: trunk/blender/source/blender/blenlib/intern/threads.c
===================================================================
--- trunk/blender/source/blender/blenlib/intern/threads.c	2012-07-13 12:55:30 UTC (rev 48892)
+++ trunk/blender/source/blender/blenlib/intern/threads.c	2012-07-13 13:47:13 UTC (rev 48893)
@@ -106,7 +106,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 _image_draw_lock = PTHREAD_MUTEX_INITIALIZER;
 static pthread_mutex_t _viewer_lock = PTHREAD_MUTEX_INITIALIZER;
 static pthread_mutex_t _custom1_lock = PTHREAD_MUTEX_INITIALIZER;
 static pthread_mutex_t _rcache_lock = PTHREAD_MUTEX_INITIALIZER;
@@ -337,8 +337,8 @@
 {
 	if (type == LOCK_IMAGE)
 		pthread_mutex_lock(&_image_lock);
-	else if (type == LOCK_PREVIEW)
-		pthread_mutex_lock(&_preview_lock);
+	else if (type == LOCK_DRAW_IMAGE)
+		pthread_mutex_lock(&_image_draw_lock);
 	else if (type == LOCK_VIEWER)
 		pthread_mutex_lock(&_viewer_lock);
 	else if (type == LOCK_CUSTOM1)
@@ -357,8 +357,8 @@
 {
 	if (type == LOCK_IMAGE)
 		pthread_mutex_unlock(&_image_lock);
-	else if (type == LOCK_PREVIEW)
-		pthread_mutex_unlock(&_preview_lock);
+	else if (type == LOCK_DRAW_IMAGE)
+		pthread_mutex_unlock(&_image_draw_lock);
 	else if (type == LOCK_VIEWER)
 		pthread_mutex_unlock(&_viewer_lock);
 	else if (type == LOCK_CUSTOM1)

Modified: trunk/blender/source/blender/compositor/operations/COM_CompositorOperation.cpp
===================================================================
--- trunk/blender/source/blender/compositor/operations/COM_CompositorOperation.cpp	2012-07-13 12:55:30 UTC (rev 48892)
+++ trunk/blender/source/blender/compositor/operations/COM_CompositorOperation.cpp	2012-07-13 13:47:13 UTC (rev 48893)
@@ -27,6 +27,7 @@
 #include "BKE_image.h"
 
 extern "C" {
+	#include "BLI_threads.h"
 	#include "RE_pipeline.h"
 	#include "RE_shader_ext.h"
 	#include "RE_render_ext.h"
@@ -63,6 +64,7 @@
 		const RenderData *rd = this->m_rd;
 		Render *re = RE_GetRender_FromData(rd);
 		RenderResult *rr = RE_AcquireResultWrite(re);
+
 		if (rr) {
 			if (rr->rectf != NULL) {
 				MEM_freeN(rr->rectf);
@@ -75,7 +77,9 @@
 			}
 		}
 
+		BLI_lock_thread(LOCK_DRAW_IMAGE);
 		BKE_image_signal(BKE_image_verify_viewer(IMA_TYPE_R_RESULT, "Render Result"), NULL, IMA_SIGNAL_FREE);
+		BLI_unlock_thread(LOCK_DRAW_IMAGE);
 
 		if (re) {
 			RE_ReleaseResult(re);

Modified: trunk/blender/source/blender/compositor/operations/COM_ViewerBaseOperation.cpp
===================================================================
--- trunk/blender/source/blender/compositor/operations/COM_ViewerBaseOperation.cpp	2012-07-13 12:55:30 UTC (rev 48892)
+++ trunk/blender/source/blender/compositor/operations/COM_ViewerBaseOperation.cpp	2012-07-13 13:47:13 UTC (rev 48893)
@@ -62,6 +62,8 @@
 	
 	if (!ibuf) return;
 	if (ibuf->x != (int)getWidth() || ibuf->y != (int)getHeight()) {
+		BLI_lock_thread(LOCK_DRAW_IMAGE);
+
 		imb_freerectImBuf(ibuf);
 		imb_freerectfloatImBuf(ibuf);
 		IMB_freezbuffloatImBuf(ibuf);
@@ -70,6 +72,8 @@
 		imb_addrectImBuf(ibuf);
 		imb_addrectfloatImBuf(ibuf);
 		anImage->ok = IMA_OK_LOADED;
+
+		BLI_unlock_thread(LOCK_DRAW_IMAGE);
 	}
 	
 	/* now we combine the input with ibuf */

Modified: trunk/blender/source/blender/editors/space_image/image_draw.c
===================================================================
--- trunk/blender/source/blender/editors/space_image/image_draw.c	2012-07-13 12:55:30 UTC (rev 48892)
+++ trunk/blender/source/blender/editors/space_image/image_draw.c	2012-07-13 13:47:13 UTC (rev 48893)
@@ -728,11 +728,21 @@
 	/* retrieve the image and information about it */
 	ima = ED_space_image(sima);
 	ED_space_image_zoom(sima, ar, &zoomx, &zoomy);
-	ibuf = ED_space_image_acquire_buffer(sima, &lock);
 
 	show_viewer = (ima && ima->source == IMA_SRC_VIEWER);
 	show_render = (show_viewer && ima->type == IMA_TYPE_R_RESULT);
 
+	if (show_viewer) {
+		/* use locked draw for drawing viewer image buffer since the conpositor
+		 * is running in separated thread and compositor could free this buffers.
+		 * other images are not modifying in such a way so they does not require
+		 * lock (sergey)
+		 */
+		BLI_lock_thread(LOCK_DRAW_IMAGE);
+	}
+
+	ibuf = ED_space_image_acquire_buffer(sima, &lock);
+
 	/* draw the image or grid */
 	if (ibuf == NULL)
 		ED_region_grid_draw(ar, zoomx, zoomy);
@@ -770,5 +780,8 @@
 	/* render info */
 	if (ima && show_render)
 		draw_render_info(scene, ima, ar);
+
+	if (show_viewer) {
+		BLI_unlock_thread(LOCK_DRAW_IMAGE);
+	}
 }
-

Modified: trunk/blender/source/blender/editors/space_node/node_draw.c
===================================================================
--- trunk/blender/source/blender/editors/space_node/node_draw.c	2012-07-13 12:55:30 UTC (rev 48892)
+++ trunk/blender/source/blender/editors/space_node/node_draw.c	2012-07-13 13:47:13 UTC (rev 48893)
@@ -335,9 +335,6 @@
 
 	/* preview rect? */
 	if (node->flag & NODE_PREVIEW) {
-		/* only recalculate size when there's a preview actually, otherwise we use stored result */
-		BLI_lock_thread(LOCK_PREVIEW);
-
 		if (node->preview && node->preview->rect) {
 			float aspect = 1.0f;
 			
@@ -374,8 +371,6 @@
 			node->prvr.ymin = dy - oldh;
 			dy = node->prvr.ymin - NODE_DYS / 2;
 		}
-
-		BLI_unlock_thread(LOCK_PREVIEW);
 	}
 
 	/* buttons rect? */
@@ -861,10 +856,8 @@
 	
 	/* preview */
 	if (node->flag & NODE_PREVIEW) {
-		BLI_lock_thread(LOCK_PREVIEW);
 		if (node->preview && node->preview->rect && !BLI_rctf_is_empty(&node->prvr))
 			node_draw_preview(node->preview, &node->prvr);
-		BLI_unlock_thread(LOCK_PREVIEW);
 	}
 	
 	UI_ThemeClearColor(color_id);

Modified: trunk/blender/source/blender/nodes/composite/node_composite_util.c
===================================================================
--- trunk/blender/source/blender/nodes/composite/node_composite_util.c	2012-07-13 12:55:30 UTC (rev 48892)
+++ trunk/blender/source/blender/nodes/composite/node_composite_util.c	2012-07-13 13:47:13 UTC (rev 48893)
@@ -647,7 +647,7 @@
 		if (stackbuf_use!=stackbuf)
 			free_compbuf(stackbuf_use);
 
-		BLI_lock_thread(LOCK_PREVIEW);
+		// BLI_lock_thread(LOCK_PREVIEW);
 
 		if (preview->rect)
 			MEM_freeN(preview->rect);
@@ -655,7 +655,7 @@
 		preview->ysize= ysize;
 		preview->rect= rect;
 
-		BLI_unlock_thread(LOCK_PREVIEW);
+		// BLI_unlock_thread(LOCK_PREVIEW);
 	}
 }
 




More information about the Bf-blender-cvs mailing list