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 :)

3 files changed, 32 insertions, 18 deletions

diff --git a/source/blender/makesrna/intern/rna_image.c b/source/blender/makesrna/intern/rna_image.c
index f8af2427827..9fedbee41ff 100644
--- a/source/blender/makesrna/intern/rna_image.c
+++ b/source/blender/makesrna/intern/rna_image.c
@@ -113,7 +113,7 @@ static void rna_Image_fields_update(Main *UNUSED(bmain), Scene *UNUSED(scene), P
 			BKE_image_signal(ima, NULL, IMA_SIGNAL_FREE);
 	}
 
-	BKE_image_release_ibuf(ima, lock);
+	BKE_image_release_ibuf(ima, ibuf, lock);
 }
 
 static void rna_Image_free_update(Main *UNUSED(bmain), Scene *UNUSED(scene), PointerRNA *ptr)
@@ -187,8 +187,12 @@ static EnumPropertyItem *rna_Image_source_itemf(bContext *UNUSED(C), PointerRNA
 static int rna_Image_file_format_get(PointerRNA *ptr)
 {
 	Image *image = (Image *)ptr->data;
-	ImBuf *ibuf = BKE_image_get_ibuf(image, NULL);
-	return BKE_ftype_to_imtype(ibuf ? ibuf->ftype : 0);
+	ImBuf *ibuf = BKE_image_acquire_ibuf(image, NULL, NULL);
+	int imtype = BKE_ftype_to_imtype(ibuf ? ibuf->ftype : 0);
+
+	BKE_image_release_ibuf(image, ibuf, NULL);
+
+	return imtype;
 }
 
 static void rna_Image_file_format_set(PointerRNA *ptr, int value)
@@ -199,12 +203,13 @@ static void rna_Image_file_format_set(PointerRNA *ptr, int value)
 		int ftype = BKE_imtype_to_ftype(value);
 
 #if 0
-		ibuf = BKE_image_get_ibuf(image, NULL);
+		ibuf = BKE_image_acquire_ibuf(image, NULL, NULL);
 		if (ibuf)
 			ibuf->ftype = ftype;
 #endif
 
 		/* to be safe change all buffer file types */
+		/* TODO: this is never threadsafe */
 		for (ibuf = image->ibufs.first; ibuf; ibuf = ibuf->next) {
 			ibuf->ftype = ftype;
 		}
@@ -237,7 +242,7 @@ static void rna_Image_size_get(PointerRNA *ptr, int *values)
 		values[1] = 0;
 	}
 
-	BKE_image_release_ibuf(im, lock);
+	BKE_image_release_ibuf(im, ibuf, lock);
 }
 
 static void rna_Image_resolution_get(PointerRNA *ptr, float *values)
@@ -256,7 +261,7 @@ static void rna_Image_resolution_get(PointerRNA *ptr, float *values)
 		values[1] = 0;
 	}
 
-	BKE_image_release_ibuf(im, lock);
+	BKE_image_release_ibuf(im, ibuf, lock);
 }
 
 static void rna_Image_resolution_set(PointerRNA *ptr, const float *values)
@@ -271,7 +276,7 @@ static void rna_Image_resolution_set(PointerRNA *ptr, const float *values)
 		ibuf->ppm[1] = values[1];
 	}
 
-	BKE_image_release_ibuf(im, lock);
+	BKE_image_release_ibuf(im, ibuf, lock);
 }
 
 static int rna_Image_depth_get(PointerRNA *ptr)
@@ -290,7 +295,7 @@ static int rna_Image_depth_get(PointerRNA *ptr)
 	else
 		planes = ibuf->planes;
 
-	BKE_image_release_ibuf(im, lock);
+	BKE_image_release_ibuf(im, ibuf, lock);
 
 	return planes;
 }
@@ -317,7 +322,7 @@ static int rna_Image_pixels_get_length(PointerRNA *ptr, int length[RNA_MAX_ARRAY
 	else
 		length[0] = 0;
 
-	BKE_image_release_ibuf(ima, lock);
+	BKE_image_release_ibuf(ima, ibuf, lock);
 
 	return length[0];
 }
@@ -343,7 +348,7 @@ static void rna_Image_pixels_get(PointerRNA *ptr, float *values)
 		}
 	}
 
-	BKE_image_release_ibuf(ima, lock);
+	BKE_image_release_ibuf(ima, ibuf, lock);
 }
 
 static void rna_Image_pixels_set(PointerRNA *ptr, const float *values)
@@ -369,7 +374,7 @@ static void rna_Image_pixels_set(PointerRNA *ptr, const float *values)
 		ibuf->userflags |= IB_BITMAPDIRTY | IB_DISPLAY_BUFFER_INVALID;
 	}
 
-	BKE_image_release_ibuf(ima, lock);
+	BKE_image_release_ibuf(ima, ibuf, lock);
 }
 
 #else
diff --git a/source/blender/makesrna/intern/rna_image_api.c b/source/blender/makesrna/intern/rna_image_api.c
index cb5c515ce1f..686e6c80f1d 100644
--- a/source/blender/makesrna/intern/rna_image_api.c
+++ b/source/blender/makesrna/intern/rna_image_api.c
@@ -99,7 +99,7 @@ static void rna_Image_save_render(Image *image, bContext *C, ReportList *reports
 				IMB_freeImBuf(write_ibuf);
 		}
 
-		BKE_image_release_ibuf(image, lock);
+		BKE_image_release_ibuf(image, ibuf, lock);
 	}
 	else {
 		BKE_report(reports, RPT_ERROR, "Scene not in context, could not get save parameters");
@@ -108,7 +108,7 @@ static void rna_Image_save_render(Image *image, bContext *C, ReportList *reports
 
 static void rna_Image_save(Image *image, ReportList *reports)
 {
-	ImBuf *ibuf = BKE_image_get_ibuf(image, NULL);
+	ImBuf *ibuf = BKE_image_acquire_ibuf(image, NULL, NULL);
 	if (ibuf) {
 		char filename[FILE_MAX];
 		BLI_strncpy(filename, image->name, sizeof(filename));
@@ -136,11 +136,13 @@ static void rna_Image_save(Image *image, ReportList *reports)
 	else {
 		BKE_reportf(reports, RPT_ERROR, "Image '%s' does not have any image data", image->id.name + 2);
 	}
+
+	BKE_image_release_ibuf(image, ibuf, NULL);
 }
 
 static void rna_Image_pack(Image *image, ReportList *reports, int as_png)
 {
-	ImBuf *ibuf = BKE_image_get_ibuf(image, NULL);
+	ImBuf *ibuf = BKE_image_acquire_ibuf(image, NULL, NULL);
 
 	if (!as_png && (ibuf && (ibuf->userflags & IB_BITMAPDIRTY))) {
 		BKE_report(reports, RPT_ERROR, "Cannot pack edited image from disk, only as internal PNG");
@@ -153,6 +155,8 @@ static void rna_Image_pack(Image *image, ReportList *reports, int as_png)
 			image->packedfile = newPackedFile(reports, image->name, ID_BLEND_PATH(G.main, &image->id));
 		}
 	}
+
+	BKE_image_release_ibuf(image, ibuf, NULL);
 }
 
 static void rna_Image_unpack(Image *image, ReportList *reports, int method)
@@ -177,7 +181,7 @@ static void rna_Image_reload(Image *image)
 
 static void rna_Image_update(Image *image, ReportList *reports)
 {
-	ImBuf *ibuf = BKE_image_get_ibuf(image, NULL);
+	ImBuf *ibuf = BKE_image_acquire_ibuf(image, NULL, NULL);
 
 	if (ibuf == NULL) {
 		BKE_reportf(reports, RPT_ERROR, "Image '%s' does not have any image data", image->id.name + 2);
@@ -188,6 +192,8 @@ static void rna_Image_update(Image *image, ReportList *reports)
 		IMB_rect_from_float(ibuf);
 
 	ibuf->userflags |= IB_DISPLAY_BUFFER_INVALID;
+
+	BKE_image_release_ibuf(image, ibuf, NULL);
 }
 
 static void rna_Image_scale(Image *image, ReportList *reports, int width, int height)
@@ -206,10 +212,11 @@ static int rna_Image_gl_load(Image *image, ReportList *reports, int filter, int
 	if (*bind)
 		return error;
 
-	ibuf = BKE_image_get_ibuf(image, NULL);
+	ibuf = BKE_image_acquire_ibuf(image, NULL, NULL);
 
 	if (ibuf == NULL || ibuf->rect == NULL) {
 		BKE_reportf(reports, RPT_ERROR, "Image '%s' does not have any image data", image->id.name + 2);
+		BKE_image_release_ibuf(image, ibuf, NULL);
 		return (int)GL_INVALID_OPERATION;
 	}
 
@@ -238,6 +245,8 @@ static int rna_Image_gl_load(Image *image, ReportList *reports, int filter, int
 		image->bindcode = 0;
 	}
 
+	BKE_image_release_ibuf(image, ibuf, NULL);
+
 	return error;
 }
 
diff --git a/source/blender/makesrna/intern/rna_space.c b/source/blender/makesrna/intern/rna_space.c
index 962a92223eb..caa81dafa0e 100644
--- a/source/blender/makesrna/intern/rna_space.c
+++ b/source/blender/makesrna/intern/rna_space.c
@@ -598,7 +598,7 @@ static EnumPropertyItem *rna_SpaceImageEditor_draw_channels_itemf(bContext *UNUS
 	alpha = ibuf && (ibuf->channels == 4);
 	zbuf = ibuf && (ibuf->zbuf || ibuf->zbuf_float || (ibuf->channels == 1));
 
-	ED_space_image_release_buffer(sima, lock);
+	ED_space_image_release_buffer(sima, ibuf, lock);
 
 	if (alpha && zbuf)
 		return draw_channels_items;
@@ -678,7 +678,7 @@ static void rna_SpaceImageEditor_scopes_update(Main *UNUSED(bmain), Scene *scene
 		scopes_update(&sima->scopes, ibuf, &scene->view_settings, &scene->display_settings);
 		WM_main_add_notifier(NC_IMAGE, sima->image);
 	}
-	ED_space_image_release_buffer(sima, lock);
+	ED_space_image_release_buffer(sima, ibuf, lock);
 }
 
 /* Space Text Editor */