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, 30 insertions, 16 deletions

diff --git a/source/blender/editors/object/object_bake.c b/source/blender/editors/object/object_bake.c
index 032314d06f3..e144c38a350 100644
--- a/source/blender/editors/object/object_bake.c
+++ b/source/blender/editors/object/object_bake.c
@@ -114,7 +114,7 @@ typedef struct {
 } MultiresBakeRender;
 
 typedef void (*MPassKnownData)(DerivedMesh *lores_dm, DerivedMesh *hires_dm, const void *bake_data,
-                               const int face_index, const int lvl, const float st[2],
+                               ImBuf *ibuf, const int face_index, const int lvl, const float st[2],
                                float tangmat[3][3], const int x, const int y);
 
 typedef void * (*MInitBakeData)(MultiresBakeRender *bkr, Image *ima);
@@ -133,6 +133,7 @@ typedef struct {
 	DerivedMesh *lores_dm, *hires_dm;
 	int lvl;
 	void *bake_data;
+	ImBuf *ibuf;
 	MPassKnownData pass_data;
 } MResolvePixelData;
 
@@ -257,7 +258,7 @@ static void flush_pixel(const MResolvePixelData *data, const int x, const int y)
 	/* sequence end */
 
 	data->pass_data(data->lores_dm, data->hires_dm, data->bake_data,
-	                data->face_index, data->lvl, st, to_tang, x, y);
+	                data->ibuf, data->face_index, data->lvl, st, to_tang, x, y);
 }
 
 static void set_rast_triangle(const MBakeRast *bake_rast, const int x, const int y)
@@ -368,7 +369,7 @@ static void do_multires_bake(MultiresBakeRender *bkr, Image *ima, MPassKnownData
                              MInitBakeData initBakeData, MApplyBakeData applyBakeData, MFreeBakeData freeBakeData)
 {
 	DerivedMesh *dm = bkr->lores_dm;
-	ImBuf *ibuf = BKE_image_get_ibuf(ima, NULL);
+	ImBuf *ibuf = BKE_image_acquire_ibuf(ima, NULL, NULL);
 	const int lvl = bkr->lvl;
 	const int tot_face = dm->getNumTessFaces(dm);
 	MVert *mvert = dm->getVertArray(dm);
@@ -414,6 +415,7 @@ static void do_multires_bake(MultiresBakeRender *bkr, Image *ima, MPassKnownData
 				continue;
 
 			data.face_index = f;
+			data.ibuf = ibuf;
 
 			/* might support other forms of diagonal splits later on such as
 			 * split by shortest diagonal.*/
@@ -449,6 +451,8 @@ static void do_multires_bake(MultiresBakeRender *bkr, Image *ima, MPassKnownData
 		if (freeBakeData)
 			freeBakeData(data.bake_data);
 	}
+
+	BKE_image_release_ibuf(ima, ibuf, NULL);
 }
 
 /* mode = 0: interpolate normals,
@@ -579,7 +583,7 @@ static void interp_barycentric_mface(DerivedMesh *dm, MFace *mface, const float
 static void *init_heights_data(MultiresBakeRender *bkr, Image *ima)
 {
 	MHeightBakeData *height_data;
-	ImBuf *ibuf = BKE_image_get_ibuf(ima, NULL);
+	ImBuf *ibuf = BKE_image_acquire_ibuf(ima, NULL, NULL);
 	DerivedMesh *lodm = bkr->lores_dm;
 
 	height_data = MEM_callocN(sizeof(MHeightBakeData), "MultiresBake heightData");
@@ -609,6 +613,8 @@ static void *init_heights_data(MultiresBakeRender *bkr, Image *ima)
 	height_data->orig_index_mf_to_mpoly = lodm->getTessFaceDataArray(lodm, CD_ORIGINDEX);
 	height_data->orig_index_mp_to_orig = lodm->getPolyDataArray(lodm, CD_ORIGINDEX);
 
+	BKE_image_release_ibuf(ima, ibuf, NULL);
+
 	return (void *)height_data;
 }
 
@@ -635,7 +641,7 @@ static void free_normal_data(void *bake_data)
 static void apply_heights_data(void *bake_data)
 {
 	MHeightBakeData *height_data = (MHeightBakeData *)bake_data;
-	ImBuf *ibuf = BKE_image_get_ibuf(height_data->ima, NULL);
+	ImBuf *ibuf = BKE_image_acquire_ibuf(height_data->ima, NULL, NULL);
 	int x, y, i;
 	float height, *heights = height_data->heights;
 	float min = height_data->height_min, max = height_data->height_max;
@@ -667,6 +673,8 @@ static void apply_heights_data(void *bake_data)
 	}
 
 	ibuf->userflags = IB_RECT_INVALID | IB_DISPLAY_BUFFER_INVALID;
+
+	BKE_image_release_ibuf(height_data->ima, ibuf, NULL);
 }
 
 static void free_heights_data(void *bake_data)
@@ -687,13 +695,11 @@ static void free_heights_data(void *bake_data)
  *     mesh to make texture smoother) let's call this point p0 and n.
  *   - height wound be dot(n, p1-p0) */
 static void apply_heights_callback(DerivedMesh *lores_dm, DerivedMesh *hires_dm, const void *bake_data,
-                                   const int face_index, const int lvl, const float st[2],
+                                   ImBuf *ibuf, const int face_index, const int lvl, const float st[2],
                                    float UNUSED(tangmat[3][3]), const int x, const int y)
 {
 	MTFace *mtface = CustomData_get_layer(&lores_dm->faceData, CD_MTFACE);
 	MFace mface;
-	Image *ima = mtface[face_index].tpage;
-	ImBuf *ibuf = BKE_image_get_ibuf(ima, NULL);
 	MHeightBakeData *height_data = (MHeightBakeData *)bake_data;
 	float uv[2], *st0, *st1, *st2, *st3;
 	int pixel = ibuf->x * y + x;
@@ -764,13 +770,11 @@ static void apply_heights_callback(DerivedMesh *lores_dm, DerivedMesh *hires_dm,
  *   - multiply it by tangmat
  *   - vector in color space would be norm(vec) /2 + (0.5, 0.5, 0.5) */
 static void apply_tangmat_callback(DerivedMesh *lores_dm, DerivedMesh *hires_dm, const void *bake_data,
-                                   const int face_index, const int lvl, const float st[2],
+                                   ImBuf *ibuf, const int face_index, const int lvl, const float st[2],
                                    float tangmat[3][3], const int x, const int y)
 {
 	MTFace *mtface = CustomData_get_layer(&lores_dm->faceData, CD_MTFACE);
 	MFace mface;
-	Image *ima = mtface[face_index].tpage;
-	ImBuf *ibuf = BKE_image_get_ibuf(ima, NULL);
 	MNormalBakeData *normal_data = (MNormalBakeData *)bake_data;
 	float uv[2], *st0, *st1, *st2, *st3;
 	int pixel = ibuf->x * y + x;
@@ -853,7 +857,7 @@ static void bake_images(MultiresBakeRender *bkr)
 
 	for (link = bkr->image.first; link; link = link->next) {
 		Image *ima = (Image *)link->data;
-		ImBuf *ibuf = BKE_image_get_ibuf(ima, NULL);
+		ImBuf *ibuf = BKE_image_acquire_ibuf(ima, NULL, NULL);
 
 		if (ibuf->x > 0 && ibuf->y > 0) {
 			ibuf->userdata = MEM_callocN(ibuf->y * ibuf->x, "MultiresBake imbuf mask");
@@ -869,6 +873,8 @@ static void bake_images(MultiresBakeRender *bkr)
 			}
 		}
 
+		BKE_image_release_ibuf(ima, ibuf, NULL);
+
 		ima->id.flag |= LIB_DOIT;
 	}
 }
@@ -879,7 +885,7 @@ static void finish_images(MultiresBakeRender *bkr)
 
 	for (link = bkr->image.first; link; link = link->next) {
 		Image *ima = (Image *)link->data;
-		ImBuf *ibuf = BKE_image_get_ibuf(ima, NULL);
+		ImBuf *ibuf = BKE_image_acquire_ibuf(ima, NULL, NULL);
 
 		if (ibuf->x <= 0 || ibuf->y <= 0)
 			continue;
@@ -900,6 +906,8 @@ static void finish_images(MultiresBakeRender *bkr)
 			MEM_freeN(ibuf->userdata);
 			ibuf->userdata = NULL;
 		}
+
+		BKE_image_release_ibuf(ima, ibuf, NULL);
 	}
 }
 
@@ -974,7 +982,7 @@ static int multiresbake_check(bContext *C, wmOperator *op)
 					ok = 0;
 				}
 				else {
-					ImBuf *ibuf = BKE_image_get_ibuf(ima, NULL);
+					ImBuf *ibuf = BKE_image_acquire_ibuf(ima, NULL, NULL);
 
 					if (!ibuf) {
 						BKE_report(op->reports, RPT_ERROR, "Baking should happen to image with image buffer");
@@ -991,6 +999,8 @@ static int multiresbake_check(bContext *C, wmOperator *op)
 						if (!ok)
 							BKE_report(op->reports, RPT_ERROR, "Baking to unsupported image type");
 					}
+
+					BKE_image_release_ibuf(ima, ibuf, NULL);
 				}
 			}
 		}
@@ -1061,10 +1071,12 @@ static void clear_images(MTFace *mtface, int totface)
 		Image *ima = mtface[a].tpage;
 
 		if ((ima->id.flag & LIB_DOIT) == 0) {
-			ImBuf *ibuf = BKE_image_get_ibuf(ima, NULL);
+			ImBuf *ibuf = BKE_image_acquire_ibuf(ima, NULL, NULL);
 
 			IMB_rectfill(ibuf, (ibuf->planes == R_IMF_PLANES_RGBA) ? vec_alpha : vec_solid);
 			ima->id.flag |= LIB_DOIT;
+
+			BKE_image_release_ibuf(ima, ibuf, NULL);
 		}
 	}
 
@@ -1358,7 +1370,7 @@ static void finish_bake_internal(BakeRender *bkr)
 
 	/* force OpenGL reload and mipmap recalc */
 	for (ima = G.main->image.first; ima; ima = ima->id.next) {
-		ImBuf *ibuf = BKE_image_get_ibuf(ima, NULL);
+		ImBuf *ibuf = BKE_image_acquire_ibuf(ima, NULL, NULL);
 
 		/* some of the images could have been changed during bake,
 		 * so recreate mipmaps regardless bake result status
@@ -1383,6 +1395,8 @@ static void finish_bake_internal(BakeRender *bkr)
 				ibuf->userdata = NULL;
 			}
 		}
+
+		BKE_image_release_ibuf(ima, ibuf, NULL);
 	}
 }