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

5 files changed, 54 insertions, 35 deletions

diff --git a/source/blender/editors/space_image/image_buttons.c b/source/blender/editors/space_image/image_buttons.c
index ae7a718ab5b..0a3db59096a 100644
--- a/source/blender/editors/space_image/image_buttons.c
+++ b/source/blender/editors/space_image/image_buttons.c
@@ -593,7 +593,7 @@ void uiTemplateImage(uiLayout *layout, bContext *C, PointerRNA *ptr, const char
 		if (ima->source == IMA_SRC_VIEWER) {
 			ibuf = BKE_image_acquire_ibuf(ima, iuser, &lock);
 			image_info(scene, iuser, ima, ibuf, str);
-			BKE_image_release_ibuf(ima, lock);
+			BKE_image_release_ibuf(ima, ibuf, lock);
 
 			uiItemL(layout, ima->id.name + 2, ICON_NONE);
 			uiItemL(layout, str, ICON_NONE);
@@ -663,7 +663,7 @@ void uiTemplateImage(uiLayout *layout, bContext *C, PointerRNA *ptr, const char
 				if (compact == 0) {
 					ibuf = BKE_image_acquire_ibuf(ima, iuser, &lock);
 					image_info(scene, iuser, ima, ibuf, str);
-					BKE_image_release_ibuf(ima, lock);
+					BKE_image_release_ibuf(ima, ibuf, lock);
 					uiItemL(layout, str, ICON_NONE);
 				}
 			}
@@ -722,7 +722,6 @@ void uiTemplateImage(uiLayout *layout, bContext *C, PointerRNA *ptr, const char
 				split = uiLayoutSplit(layout, 0.0f, FALSE);
 
 				col = uiLayoutColumn(split, TRUE);
-				uiLayoutSetEnabled(col, 0);
 				uiItemR(col, &imaptr, "generated_width", 0, "X", ICON_NONE);
 				uiItemR(col, &imaptr, "generated_height", 0, "Y", ICON_NONE);
 				
diff --git a/source/blender/editors/space_image/image_draw.c b/source/blender/editors/space_image/image_draw.c
index 9fc2b981547..d565e6f9e9a 100644
--- a/source/blender/editors/space_image/image_draw.c
+++ b/source/blender/editors/space_image/image_draw.c
@@ -652,7 +652,7 @@ static unsigned char *get_alpha_clone_image(const bContext *C, Scene *scene, int
 	if (!brush || !brush->clone.image)
 		return NULL;
 	
-	ibuf = BKE_image_get_ibuf(brush->clone.image, NULL);
+	ibuf = BKE_image_acquire_ibuf(brush->clone.image, NULL, NULL);
 
 	if (!ibuf)
 		return NULL;
@@ -660,6 +660,7 @@ static unsigned char *get_alpha_clone_image(const bContext *C, Scene *scene, int
 	display_buffer = IMB_display_buffer_acquire_ctx(C, ibuf, &cache_handle);
 
 	if (!display_buffer) {
+		BKE_image_release_ibuf(brush->clone.image, ibuf, NULL);
 		IMB_display_buffer_release(cache_handle);
 
 		return NULL;
@@ -669,8 +670,10 @@ static unsigned char *get_alpha_clone_image(const bContext *C, Scene *scene, int
 
 	IMB_display_buffer_release(cache_handle);
 
-	if (!rect)
+	if (!rect) {
+		BKE_image_release_ibuf(brush->clone.image, ibuf, NULL);
 		return NULL;
+	}
 
 	*width = ibuf->x;
 	*height = ibuf->y;
@@ -684,6 +687,8 @@ static unsigned char *get_alpha_clone_image(const bContext *C, Scene *scene, int
 		cp += 4;
 	}
 
+	BKE_image_release_ibuf(brush->clone.image, ibuf, NULL);
+
 	return rect;
 }
 
@@ -799,7 +804,7 @@ void draw_image_main(const bContext *C, ARegion *ar)
 	}
 #endif
 
-	ED_space_image_release_buffer(sima, lock);
+	ED_space_image_release_buffer(sima, ibuf, lock);
 
 	if (show_viewer) {
 		BLI_unlock_thread(LOCK_DRAW_IMAGE);
diff --git a/source/blender/editors/space_image/image_edit.c b/source/blender/editors/space_image/image_edit.c
index 4ca2f8888f8..c4e2230e7a7 100644
--- a/source/blender/editors/space_image/image_edit.c
+++ b/source/blender/editors/space_image/image_edit.c
@@ -117,8 +117,12 @@ ImBuf *ED_space_image_acquire_buffer(SpaceImage *sima, void **lock_r)
 #endif
 		ibuf = BKE_image_acquire_ibuf(sima->image, &sima->iuser, lock_r);
 
-		if (ibuf && (ibuf->rect || ibuf->rect_float))
-			return ibuf;
+		if (ibuf) {
+			if (ibuf->rect || ibuf->rect_float)
+				return ibuf;
+
+			BKE_image_release_ibuf(sima->image, ibuf, NULL);
+		}
 	}
 	else
 		*lock_r = NULL;
@@ -126,10 +130,10 @@ ImBuf *ED_space_image_acquire_buffer(SpaceImage *sima, void **lock_r)
 	return NULL;
 }
 
-void ED_space_image_release_buffer(SpaceImage *sima, void *lock)
+void ED_space_image_release_buffer(SpaceImage *sima, ImBuf *ibuf, void *lock)
 {
 	if (sima && sima->image)
-		BKE_image_release_ibuf(sima->image, lock);
+		BKE_image_release_ibuf(sima->image, ibuf, lock);
 }
 
 int ED_space_image_has_buffer(SpaceImage *sima)
@@ -140,7 +144,7 @@ int ED_space_image_has_buffer(SpaceImage *sima)
 
 	ibuf = ED_space_image_acquire_buffer(sima, &lock);
 	has_buffer = (ibuf != NULL);
-	ED_space_image_release_buffer(sima, lock);
+	ED_space_image_release_buffer(sima, ibuf, lock);
 
 	return has_buffer;
 }
@@ -175,7 +179,7 @@ void ED_space_image_get_size(SpaceImage *sima, int *width, int *height)
 		*height = IMG_SIZE_FALLBACK;
 	}
 
-	ED_space_image_release_buffer(sima, lock);
+	ED_space_image_release_buffer(sima, ibuf, lock);
 }
 
 void ED_space_image_get_size_fl(SpaceImage *sima, float size[2])
diff --git a/source/blender/editors/space_image/image_ops.c b/source/blender/editors/space_image/image_ops.c
index ce3c6e1fd26..d4f24babff6 100644
--- a/source/blender/editors/space_image/image_ops.c
+++ b/source/blender/editors/space_image/image_ops.c
@@ -167,7 +167,7 @@ static int space_image_file_exists_poll(bContext *C)
 				ret = TRUE;
 			}
 		}
-		ED_space_image_release_buffer(sima, lock);
+		ED_space_image_release_buffer(sima, ibuf, lock);
 
 		return ret;
 	}
@@ -1188,7 +1188,7 @@ static int save_image_options_init(SaveImageOptions *simopts, SpaceImage *sima,
 		BKE_color_managed_view_settings_copy(&simopts->im_format.view_settings, &scene->view_settings);
 	}
 
-	ED_space_image_release_buffer(sima, lock);
+	ED_space_image_release_buffer(sima, ibuf, lock);
 
 	return (ibuf != NULL);
 }
@@ -1328,7 +1328,7 @@ static void save_image_doit(bContext *C, SpaceImage *sima, wmOperator *op, SaveI
 			IMB_freeImBuf(colormanaged_ibuf);
 	}
 
-	ED_space_image_release_buffer(sima, lock);
+	ED_space_image_release_buffer(sima, ibuf, lock);
 }
 
 static void image_save_as_free(wmOperator *op)
@@ -1743,17 +1743,14 @@ void IMAGE_OT_new(wmOperatorType *ot)
 static int image_invert_poll(bContext *C)
 {
 	Image *ima = CTX_data_edit_image(C);
-	ImBuf *ibuf = BKE_image_get_ibuf(ima, NULL);
-	
-	if (ibuf != NULL)
-		return 1;
-	return 0;
+
+	return BKE_image_has_ibuf(ima, NULL);
 }
 
 static int image_invert_exec(bContext *C, wmOperator *op)
 {
 	Image *ima = CTX_data_edit_image(C);
-	ImBuf *ibuf = BKE_image_get_ibuf(ima, NULL);
+	ImBuf *ibuf = BKE_image_acquire_ibuf(ima, NULL, NULL);
 
 	/* flags indicate if this channel should be inverted */
 	const short r = RNA_boolean_get(op->ptr, "invert_r");
@@ -1792,6 +1789,7 @@ static int image_invert_exec(bContext *C, wmOperator *op)
 		}
 	}
 	else {
+		BKE_image_release_ibuf(ima, ibuf, NULL);
 		return OPERATOR_CANCELLED;
 	}
 
@@ -1800,6 +1798,9 @@ static int image_invert_exec(bContext *C, wmOperator *op)
 		ibuf->userflags |= IB_MIPMAP_INVALID;
 
 	WM_event_add_notifier(C, NC_IMAGE | NA_EDITED, ima);
+
+	BKE_image_release_ibuf(ima, ibuf, NULL);
+
 	return OPERATOR_FINISHED;
 }
 
@@ -1848,7 +1849,7 @@ static int image_pack_exec(bContext *C, wmOperator *op)
 {
 	struct Main *bmain = CTX_data_main(C);
 	Image *ima = CTX_data_edit_image(C);
-	ImBuf *ibuf = BKE_image_get_ibuf(ima, NULL);
+	ImBuf *ibuf = BKE_image_acquire_ibuf(ima, NULL, NULL);
 	int as_png = RNA_boolean_get(op->ptr, "as_png");
 
 	if (!image_pack_test(C, op))
@@ -1865,30 +1866,38 @@ static int image_pack_exec(bContext *C, wmOperator *op)
 		ima->packedfile = newPackedFile(op->reports, ima->name, ID_BLEND_PATH(bmain, &ima->id));
 
 	WM_event_add_notifier(C, NC_IMAGE | NA_EDITED, ima);
-	
+
+	BKE_image_release_ibuf(ima, ibuf, NULL);
+
 	return OPERATOR_FINISHED;
 }
 
 static int image_pack_invoke(bContext *C, wmOperator *op, wmEvent *UNUSED(event))
 {
 	Image *ima = CTX_data_edit_image(C);
-	ImBuf *ibuf = BKE_image_get_ibuf(ima, NULL);
+	ImBuf *ibuf;
 	uiPopupMenu *pup;
 	uiLayout *layout;
 	int as_png = RNA_boolean_get(op->ptr, "as_png");
 
 	if (!image_pack_test(C, op))
 		return OPERATOR_CANCELLED;
-	
+
+	ibuf = BKE_image_acquire_ibuf(ima, NULL, NULL);
+
 	if (!as_png && (ibuf && (ibuf->userflags & IB_BITMAPDIRTY))) {
 		pup = uiPupMenuBegin(C, "OK", ICON_QUESTION);
 		layout = uiPupMenuLayout(pup);
 		uiItemBooleanO(layout, "Can't pack edited image from disk. Pack as internal PNG?", ICON_NONE, op->idname, "as_png", 1);
 		uiPupMenuEnd(C, pup);
 
+		BKE_image_release_ibuf(ima, ibuf, NULL);
+
 		return OPERATOR_CANCELLED;
 	}
 
+	BKE_image_release_ibuf(ima, ibuf, NULL);
+
 	return image_pack_exec(C, op);
 }
 
@@ -2032,7 +2041,7 @@ int ED_space_image_color_sample(SpaceImage *sima, ARegion *ar, int mval[2], floa
 	int ret = FALSE;
 
 	if (ibuf == NULL) {
-		ED_space_image_release_buffer(sima, lock);
+		ED_space_image_release_buffer(sima, ibuf, lock);
 		return FALSE;
 	}
 
@@ -2058,7 +2067,7 @@ int ED_space_image_color_sample(SpaceImage *sima, ARegion *ar, int mval[2], floa
 		}
 	}
 
-	ED_space_image_release_buffer(sima, lock);
+	ED_space_image_release_buffer(sima, ibuf, lock);
 	return ret;
 }
 
@@ -2074,7 +2083,7 @@ static void image_sample_apply(bContext *C, wmOperator *op, wmEvent *event)
 	CurveMapping *curve_mapping = scene->view_settings.curve_mapping;
 
 	if (ibuf == NULL) {
-		ED_space_image_release_buffer(sima, lock);
+		ED_space_image_release_buffer(sima, ibuf, lock);
 		info->draw = 0;
 		return;
 	}
@@ -2175,7 +2184,7 @@ static void image_sample_apply(bContext *C, wmOperator *op, wmEvent *event)
 		info->draw = 0;
 	}
 
-	ED_space_image_release_buffer(sima, lock);
+	ED_space_image_release_buffer(sima, ibuf, lock);
 	ED_area_tag_redraw(CTX_wm_area(C));
 }
 
@@ -2266,12 +2275,12 @@ static int image_sample_line_exec(bContext *C, wmOperator *op)
 	float x1f, y1f, x2f, y2f;
 	
 	if (ibuf == NULL) {
-		ED_space_image_release_buffer(sima, lock);
+		ED_space_image_release_buffer(sima, ibuf, lock);
 		return OPERATOR_CANCELLED;
 	}
 	/* hmmmm */
 	if (ibuf->channels < 3) {
-		ED_space_image_release_buffer(sima, lock);
+		ED_space_image_release_buffer(sima, ibuf, lock);
 		return OPERATOR_CANCELLED;
 	}
 	
@@ -2288,7 +2297,7 @@ static int image_sample_line_exec(bContext *C, wmOperator *op)
 	/* reset y zoom */
 	hist->ymax = 1.0f;
 
-	ED_space_image_release_buffer(sima, lock);
+	ED_space_image_release_buffer(sima, ibuf, lock);
 	
 	ED_area_tag_redraw(CTX_wm_area(C));
 	
@@ -2383,11 +2392,13 @@ static int image_record_composite_apply(bContext *C, wmOperator *op)
 
 	ED_area_tag_redraw(CTX_wm_area(C));
 	
-	ibuf = BKE_image_get_ibuf(sima->image, &sima->iuser);
+	ibuf = BKE_image_acquire_ibuf(sima->image, &sima->iuser, NULL);
 	/* save memory in flipbooks */
 	if (ibuf)
 		imb_freerectfloatImBuf(ibuf);
-	
+
+	BKE_image_release_ibuf(sima->image, ibuf, NULL);
+
 	scene->r.cfra++;
 
 	return (scene->r.cfra <= rcd->efra);
diff --git a/source/blender/editors/space_image/space_image.c b/source/blender/editors/space_image/space_image.c
index 4131cbfdd0d..ea696772957 100644
--- a/source/blender/editors/space_image/space_image.c
+++ b/source/blender/editors/space_image/space_image.c
@@ -780,7 +780,7 @@ static void image_scope_area_draw(const bContext *C, ARegion *ar)
 		}
 		scopes_update(&sima->scopes, ibuf, &scene->view_settings, &scene->display_settings);
 	}
-	ED_space_image_release_buffer(sima, lock);
+	ED_space_image_release_buffer(sima, ibuf, lock);
 	
 	ED_region_panels(C, ar, 1, NULL, -1);
 }