Fix deadlock happening when using Save Buffers for render

Summary:
Issue was caused by the same tile being written twice to
the EXR file. This was happening because of partial update
of work-in-progress tiles was merging result to the final
render result in order to make color management pipeline
happy.

We need to avoid such a merges and keep memory usage as
low as possible when Save Buffers is enabled.

Now render pipeline will allocate special display buffer
in render layer which will contain combined pass in the
display space. This keeps memory usage as low as we can
do at this moment.

There's one weak thing which is changing color management
settings during rendering would lead to lossy conversion.
This is because render result's display buffer uses color
space from the time when rendering was invoked.

This is actually what was happening in previous release
already actually so not a big issue.

Reviewers: brecht

Reviewed By: brecht

Differential Revision: https://developer.blender.org/D162

1 files changed, 48 insertions, 28 deletions

diff --git a/source/blender/imbuf/intern/colormanagement.c b/source/blender/imbuf/intern/colormanagement.c
index 6ca3e1ae2a9..e5884f78b75 100644
--- a/source/blender/imbuf/intern/colormanagement.c
+++ b/source/blender/imbuf/intern/colormanagement.c
@@ -745,8 +745,8 @@ void IMB_colormanagement_display_settings_from_ctx(const bContext *C,
 	}
 }
 
-static const char *display_transform_get_colorspace_name(const ColorManagedViewSettings *view_settings,
-                                                         const ColorManagedDisplaySettings *display_settings)
+const char *IMB_colormanagement_get_display_colorspace_name(const ColorManagedViewSettings *view_settings,
+                                                            const ColorManagedDisplaySettings *display_settings)
 {
 	OCIO_ConstConfigRcPtr *config = OCIO_getCurrentConfig();
 
@@ -764,7 +764,7 @@ static const char *display_transform_get_colorspace_name(const ColorManagedViewS
 static ColorSpace *display_transform_get_colorspace(const ColorManagedViewSettings *view_settings,
                                                     const ColorManagedDisplaySettings *display_settings)
 {
-	const char *colorspace_name = display_transform_get_colorspace_name(view_settings, display_settings);
+	const char *colorspace_name = IMB_colormanagement_get_display_colorspace_name(view_settings, display_settings);
 
 	if (colorspace_name)
 		return colormanage_colorspace_get_named(colorspace_name);
@@ -1510,7 +1510,7 @@ static bool is_ibuf_rect_in_display_space(ImBuf *ibuf, const ColorManagedViewSet
 	    view_settings->gamma == 1.0f)
 	{
 		const char *from_colorspace = ibuf->rect_colorspace->name;
-		const char *to_colorspace = display_transform_get_colorspace_name(view_settings, display_settings);
+		const char *to_colorspace = IMB_colormanagement_get_display_colorspace_name(view_settings, display_settings);
 
 		if (to_colorspace && !strcmp(from_colorspace, to_colorspace))
 			return true;
@@ -2034,7 +2034,8 @@ unsigned char *IMB_display_buffer_acquire(ImBuf *ibuf, const ColorManagedViewSet
 			IMB_partial_display_buffer_update(ibuf, ibuf->rect_float, (unsigned char *) ibuf->rect,
 			                                  ibuf->x, 0, 0, applied_view_settings, display_settings,
 			                                  ibuf->invalid_rect.xmin, ibuf->invalid_rect.ymin,
-			                                  ibuf->invalid_rect.xmax, ibuf->invalid_rect.ymax);
+			                                  ibuf->invalid_rect.xmax, ibuf->invalid_rect.ymax,
+			                                  false);
 		}
 
 		BLI_rcti_init(&ibuf->invalid_rect, 0, 0, 0, 0);
@@ -2735,16 +2736,20 @@ static void partial_buffer_update_rect(ImBuf *ibuf, unsigned char *display_buffe
 }
 
 void IMB_partial_display_buffer_update(ImBuf *ibuf, const float *linear_buffer, const unsigned char *byte_buffer,
-                                       int stride, int offset_x, int offset_y, const ColorManagedViewSettings *view_settings,
+                                       int stride, int offset_x, int offset_y,
+                                       const ColorManagedViewSettings *view_settings,
                                        const ColorManagedDisplaySettings *display_settings,
-                                       int xmin, int ymin, int xmax, int ymax)
+                                       int xmin, int ymin, int xmax, int ymax,
+                                       bool copy_display_to_byte_buffer)
 {
+	ColormanageCacheViewSettings cache_view_settings;
+	ColormanageCacheDisplaySettings cache_display_settings;
+	void *cache_handle = NULL;
+	unsigned char *display_buffer = NULL;
+	int buffer_width = ibuf->x;
+
 	if (ibuf->display_buffer_flags) {
-		ColormanageCacheViewSettings cache_view_settings;
-		ColormanageCacheDisplaySettings cache_display_settings;
-		void *cache_handle = NULL;
-		unsigned char *display_buffer = NULL;
-		int view_flag, display_index, buffer_width;
+		int view_flag, display_index;
 
 		colormanage_view_settings_to_cache(ibuf, &cache_view_settings, view_settings);
 		colormanage_display_settings_to_cache(&cache_display_settings, display_settings);
@@ -2753,6 +2758,7 @@ void IMB_partial_display_buffer_update(ImBuf *ibuf, const float *linear_buffer,
 		display_index = cache_display_settings.display - 1;
 
 		BLI_lock_thread(LOCK_COLORMANAGE);
+
 		if ((ibuf->userflags & IB_DISPLAY_BUFFER_INVALID) == 0)
 			display_buffer = colormanage_cache_get(ibuf, &cache_view_settings, &cache_display_settings, &cache_handle);
 
@@ -2767,28 +2773,42 @@ void IMB_partial_display_buffer_update(ImBuf *ibuf, const float *linear_buffer,
 		ibuf->display_buffer_flags[display_index] |= view_flag;
 
 		BLI_unlock_thread(LOCK_COLORMANAGE);
+	}
 
-		if (display_buffer) {
-			ColormanageProcessor *cm_processor = NULL;
-			bool skip_transform = false;
+	if (display_buffer == NULL) {
+		if (copy_display_to_byte_buffer) {
+			display_buffer = (unsigned char *) ibuf->rect;
+		}
+	}
 
-			/* byte buffer is assumed to be in imbuf's rect space, so if byte buffer
-			 * is known we could skip display->linear->display conversion in case
-			 * display color space matches imbuf's rect space
-			 */
-			if (byte_buffer != NULL)
-				skip_transform = is_ibuf_rect_in_display_space(ibuf, view_settings, display_settings);
+	if (display_buffer) {
+		ColormanageProcessor *cm_processor = NULL;
+		bool skip_transform = false;
+
+		/* byte buffer is assumed to be in imbuf's rect space, so if byte buffer
+		 * is known we could skip display->linear->display conversion in case
+		 * display color space matches imbuf's rect space
+		 */
+		if (byte_buffer != NULL)
+			skip_transform = is_ibuf_rect_in_display_space(ibuf, view_settings, display_settings);
 
-			if (!skip_transform)
-				cm_processor = IMB_colormanagement_display_processor_new(view_settings, display_settings);
+		if (!skip_transform)
+			cm_processor = IMB_colormanagement_display_processor_new(view_settings, display_settings);
 
-			partial_buffer_update_rect(ibuf, display_buffer, linear_buffer, byte_buffer, buffer_width, stride,
-			                           offset_x, offset_y, cm_processor, xmin, ymin, xmax, ymax);
+		partial_buffer_update_rect(ibuf, display_buffer, linear_buffer, byte_buffer, buffer_width, stride,
+		                           offset_x, offset_y, cm_processor, xmin, ymin, xmax, ymax);
 
-			if (cm_processor)
-				IMB_colormanagement_processor_free(cm_processor);
+		if (cm_processor)
+			IMB_colormanagement_processor_free(cm_processor);
 
-			IMB_display_buffer_release(cache_handle);
+		IMB_display_buffer_release(cache_handle);
+	}
+
+	if (copy_display_to_byte_buffer && (unsigned char *) ibuf->rect != display_buffer) {
+		int y;
+		for (y = ymin; y < ymax; y++) {
+			int index = y * buffer_width * 4;
+			memcpy(ibuf->rect + index, display_buffer + index, (xmax - xmin) * 4);
 		}
 	}
 }