Image thread safe improvements

This commit makes BKE_image_acquire_ibuf referencing result, which means once
some area requested for image buffer, it'll be guaranteed this buffer wouldn't
be freed by image signal.

To de-reference buffer BKE_image_release_ibuf should now always be used.

To make referencing working correct we can not rely on result of
image_get_ibuf_threadsafe called outside from thread lock. This is so because
we need to guarantee getting image buffer from list of loaded buffers and it's
referencing happens atomic. Without lock here it is possible that between call
of image_get_ibuf_threadsafe and referencing the buffer IMA_SIGNAL_FREE would
be called. Image signal handling too is blocking now to prevent such a
situation.

Threads are locking by spinlock, which are faster than mutexes. There were some
slowdown reports in the past about render slowdown when using OSX on Xeon CPU.
It shouldn't happen with spin locks, but more tests on different hardware would
be really welcome. So far can not see speed regressions on own computers.

This commit also removes BKE_image_get_ibuf, because it was not so intuitive
when get_ibuf and acquire_ibuf should be used.

Thanks to Ton and Brecht for discussion/review :)

1 files changed, 52 insertions, 20 deletions

diff --git a/source/blender/editors/sculpt_paint/paint_image.c b/source/blender/editors/sculpt_paint/paint_image.c
index 4b400623920..192ad35109a 100644
--- a/source/blender/editors/sculpt_paint/paint_image.c
+++ b/source/blender/editors/sculpt_paint/paint_image.c
@@ -500,7 +500,7 @@ static void image_undo_restore(bContext *C, ListBase *lb)
 			ima = BLI_findstring(&bmain->image, tile->idname, offsetof(ID, name));
 		}
 
-		ibuf = BKE_image_get_ibuf(ima, NULL);
+		ibuf = BKE_image_acquire_ibuf(ima, NULL, NULL);
 
 		if (ima && ibuf && strcmp(tile->ibufname, ibuf->name) != 0) {
 			/* current ImBuf filename was changed, probably current frame
@@ -508,19 +508,27 @@ static void image_undo_restore(bContext *C, ListBase *lb)
 			 * full image user (which isn't so obvious, btw) try to find ImBuf with
 			 * matched file name in list of already loaded images */
 
+			BKE_image_release_ibuf(ima, ibuf, NULL);
+
 			ibuf = BLI_findstring(&ima->ibufs, tile->ibufname, offsetof(ImBuf, name));
 		}
 
-		if (!ima || !ibuf || !(ibuf->rect || ibuf->rect_float))
+		if (!ima || !ibuf || !(ibuf->rect || ibuf->rect_float)) {
+			BKE_image_release_ibuf(ima, ibuf, NULL);
 			continue;
+		}
 
-		if (ima->gen_type != tile->gen_type || ima->source != tile->source)
+		if (ima->gen_type != tile->gen_type || ima->source != tile->source) {
+			BKE_image_release_ibuf(ima, ibuf, NULL);
 			continue;
+		}
 
 		use_float = ibuf->rect_float ? 1 : 0;
 
-		if (use_float != tile->use_float)
+		if (use_float != tile->use_float) {
+			BKE_image_release_ibuf(ima, ibuf, NULL);
 			continue;
+		}
 
 		undo_copy_tile(tile, tmpibuf, ibuf, 1);
 
@@ -530,6 +538,8 @@ static void image_undo_restore(bContext *C, ListBase *lb)
 		if (ibuf->mipmap[0])
 			ibuf->userflags |= IB_MIPMAP_INVALID;  /* force mipmap recreatiom */
 		ibuf->userflags |= IB_DISPLAY_BUFFER_INVALID;
+
+		BKE_image_release_ibuf(ima, ibuf, NULL);
 	}
 
 	IMB_freeImBuf(tmpibuf);
@@ -1398,8 +1408,8 @@ static float project_paint_uvpixel_mask(
 		Image *other_tpage = project_paint_face_image(ps, ps->dm_mtface_stencil, face_index);
 		const MTFace *tf_other = ps->dm_mtface_stencil + face_index;
 		
-		if (other_tpage && (ibuf_other = BKE_image_get_ibuf(other_tpage, NULL))) {
-			/* BKE_image_get_ibuf - TODO - this may be slow */
+		if (other_tpage && (ibuf_other = BKE_image_acquire_ibuf(other_tpage, NULL, NULL))) {
+			/* BKE_image_acquire_ibuf - TODO - this may be slow */
 			unsigned char rgba_ub[4];
 			float rgba_f[4];
 			
@@ -1411,7 +1421,9 @@ static float project_paint_uvpixel_mask(
 			else { /* from char to float */
 				mask = ((rgba_ub[0] + rgba_ub[1] + rgba_ub[2]) / (256 * 3.0f)) * (rgba_ub[3] / 256.0f);
 			}
-			
+
+			BKE_image_release_ibuf(other_tpage, ibuf_other, NULL);
+
 			if (!ps->do_layer_stencil_inv) /* matching the gimps layer mask black/white rules, white==full opacity */
 				mask = (1.0f - mask);
 
@@ -1579,8 +1591,8 @@ static ProjPixel *project_paint_uvpixel_init(
 			Image *other_tpage = project_paint_face_image(ps, ps->dm_mtface_clone, face_index);
 			const MTFace *tf_other = ps->dm_mtface_clone + face_index;
 			
-			if (other_tpage && (ibuf_other = BKE_image_get_ibuf(other_tpage, NULL))) {
-				/* BKE_image_get_ibuf - TODO - this may be slow */
+			if (other_tpage && (ibuf_other = BKE_image_acquire_ibuf(other_tpage, NULL, NULL))) {
+				/* BKE_image_acquire_ibuf - TODO - this may be slow */
 				
 				if (ibuf->rect_float) {
 					if (ibuf_other->rect_float) { /* from float to float */
@@ -1602,6 +1614,8 @@ static ProjPixel *project_paint_uvpixel_init(
 						project_face_pixel(tf_other, ibuf_other, w, side, ((ProjPixelClone *)projPixel)->clonepx.ch, NULL);
 					}
 				}
+
+				BKE_image_release_ibuf(other_tpage, ibuf_other, NULL);
 			}
 			else {
 				if (ibuf->rect_float) {
@@ -3408,7 +3422,7 @@ static void project_paint_begin(ProjPaintState *ps)
 				
 				image_index = BLI_linklist_index(image_LinkList, tpage);
 				
-				if (image_index == -1 && BKE_image_get_ibuf(tpage, NULL)) { /* MemArena dosnt have an append func */
+				if (image_index == -1 && BKE_image_has_ibuf(tpage, NULL)) { /* MemArena dosnt have an append func */
 					BLI_linklist_append(&image_LinkList, tpage);
 					image_index = ps->image_tot;
 					ps->image_tot++;
@@ -3431,7 +3445,7 @@ static void project_paint_begin(ProjPaintState *ps)
 	for (node = image_LinkList, i = 0; node; node = node->next, i++, projIma++) {
 		projIma->ima = node->link;
 		projIma->touch = 0;
-		projIma->ibuf = BKE_image_get_ibuf(projIma->ima, NULL);
+		projIma->ibuf = BKE_image_acquire_ibuf(projIma->ima, NULL, NULL);
 		projIma->partRedrawRect =  BLI_memarena_alloc(arena, sizeof(ImagePaintPartialRedraw) * PROJ_BOUNDBOX_SQUARED);
 		memset(projIma->partRedrawRect, 0, sizeof(ImagePaintPartialRedraw) * PROJ_BOUNDBOX_SQUARED);
 	}
@@ -3458,6 +3472,7 @@ static void project_paint_begin_clone(ProjPaintState *ps, int mouse[2])
 static void project_paint_end(ProjPaintState *ps)
 {
 	int a;
+	ProjPaintImage *projIma;
 	
 	/* build undo data from original pixel colors */
 	if (U.uiflag & USER_GLOBALUNDO) {
@@ -3545,7 +3560,14 @@ static void project_paint_end(ProjPaintState *ps)
 		if (tmpibuf_float) IMB_freeImBuf(tmpibuf_float);
 	}
 	/* done calculating undo data */
-	
+
+	/* dereference used image buffers */
+	for (a = 0, projIma = ps->projImages; a < ps->image_tot; a++, projIma++) {
+		BKE_image_release_ibuf(projIma->ima, projIma->ibuf, NULL);
+	}
+
+	BKE_image_release_ibuf(ps->reproject_image, ps->reproject_ibuf, NULL);
+
 	MEM_freeN(ps->screenCoords);
 	MEM_freeN(ps->bucketRect);
 	MEM_freeN(ps->bucketFaces);
@@ -4672,7 +4694,7 @@ static int texpaint_break_stroke(float *prevuv, float *fwuv, float *bkuv, float
 
 static int imapaint_canvas_set(ImagePaintState *s, Image *ima)
 {
-	ImBuf *ibuf = BKE_image_get_ibuf(ima, s->sima ? &s->sima->iuser : NULL);
+	ImBuf *ibuf = BKE_image_acquire_ibuf(ima, s->sima ? &s->sima->iuser : NULL, NULL);
 	
 	/* verify that we can paint and set canvas */
 	if (ima == NULL) {
@@ -4695,10 +4717,12 @@ static int imapaint_canvas_set(ImagePaintState *s, Image *ima)
 	/* set clone canvas */
 	if (s->tool == PAINT_TOOL_CLONE) {
 		ima = s->brush->clone.image;
-		ibuf = BKE_image_get_ibuf(ima, s->sima ? &s->sima->iuser : NULL);
+		ibuf = BKE_image_acquire_ibuf(ima, s->sima ? &s->sima->iuser : NULL, NULL);
 		
-		if (!ima || !ibuf || !(ibuf->rect || ibuf->rect_float))
+		if (!ima || !ibuf || !(ibuf->rect || ibuf->rect_float)) {
+			BKE_image_release_ibuf(ima, ibuf, NULL);
 			return 0;
+		}
 
 		s->clonecanvas = ibuf;
 
@@ -4713,13 +4737,15 @@ static int imapaint_canvas_set(ImagePaintState *s, Image *ima)
 	return 1;
 }
 
-static void imapaint_canvas_free(ImagePaintState *UNUSED(s))
+static void imapaint_canvas_free(ImagePaintState *s)
 {
+	BKE_image_release_ibuf(s->image, s->canvas, NULL);
+	BKE_image_release_ibuf(s->brush->clone.image, s->clonecanvas, NULL);
 }
 
 static int imapaint_paint_sub_stroke(ImagePaintState *s, BrushPainter *painter, Image *image, short texpaint, float *uv, double time, int update, float pressure)
 {
-	ImBuf *ibuf = BKE_image_get_ibuf(image, s->sima ? &s->sima->iuser : NULL);
+	ImBuf *ibuf = BKE_image_acquire_ibuf(image, s->sima ? &s->sima->iuser : NULL, NULL);
 	float pos[2];
 	int is_data;
 
@@ -4739,9 +4765,13 @@ static int imapaint_paint_sub_stroke(ImagePaintState *s, BrushPainter *painter,
 	if (BKE_brush_painter_paint(painter, imapaint_paint_op, pos, time, pressure, s, is_data == FALSE)) {
 		if (update)
 			imapaint_image_update(s->scene, s->sima, image, ibuf, texpaint);
+		BKE_image_release_ibuf(image, ibuf, NULL);
 		return 1;
 	}
-	else return 0;
+	else {
+		BKE_image_release_ibuf(image, ibuf, NULL);
+		return 0;
+	}
 }
 
 static int imapaint_paint_stroke(ViewContext *vc, ImagePaintState *s, BrushPainter *painter, short texpaint, const int prevmval[2], const int mval[2], double time, float pressure)
@@ -4759,7 +4789,7 @@ static int imapaint_paint_stroke(ViewContext *vc, ImagePaintState *s, BrushPaint
 			ImBuf *ibuf;
 			
 			newimage = imapaint_face_image(s, newfaceindex);
-			ibuf = BKE_image_get_ibuf(newimage, s->sima ? &s->sima->iuser : NULL);
+			ibuf = BKE_image_acquire_ibuf(newimage, s->sima ? &s->sima->iuser : NULL, NULL);
 
 			if (ibuf && ibuf->rect)
 				imapaint_pick_uv(s->scene, s->ob, newfaceindex, mval, newuv);
@@ -4767,6 +4797,8 @@ static int imapaint_paint_stroke(ViewContext *vc, ImagePaintState *s, BrushPaint
 				newimage = NULL;
 				newuv[0] = newuv[1] = 0.0f;
 			}
+
+			BKE_image_release_ibuf(newimage, ibuf, NULL);
 		}
 		else
 			newuv[0] = newuv[1] = 0.0f;
@@ -5890,7 +5922,7 @@ static int texture_paint_camera_project_exec(bContext *C, wmOperator *op)
 	}
 
 	ps.reproject_image = image;
-	ps.reproject_ibuf = BKE_image_get_ibuf(image, NULL);
+	ps.reproject_ibuf = BKE_image_acquire_ibuf(image, NULL, NULL);
 
 	if (ps.reproject_ibuf == NULL || ps.reproject_ibuf->rect == NULL) {
 		BKE_report(op->reports, RPT_ERROR, "Image data could not be found");