Fix T58549, T56741: HSV color picker issues with Filmic view transform.

In 2d655d3 the color picker was changed to use display space HSV values.
This works ok for a simple sRGB EOTF, but fails with view transforms like
Filmic where display space V 1.0 maps to RGB 16.292.

Instead we now use the color_picking role from the OCIO config when
converting from RGB to HSV in the color picker. This role is set to sRGB
in the default OCIO config.

This color space fits the following requirements:

* It is approximately perceptually linear, so that the HSV numbers and
  the HSV cube/circle have an intuitive distribution.
* It has the same gamut as the scene linear color space.
* Color picking values 0..1 map to scene linear values in the 0..1 range,
  so that picked albedo values are energy conserving.

1 files changed, 73 insertions, 61 deletions

diff --git a/source/blender/editors/interface/interface_region_color_picker.c b/source/blender/editors/interface/interface_region_color_picker.c
index a99bcb2b73c..7c2afaf1a92 100644
--- a/source/blender/editors/interface/interface_region_color_picker.c
+++ b/source/blender/editors/interface/interface_region_color_picker.c
@@ -111,6 +111,36 @@ void ui_color_picker_to_rgb(float r_cp0, float r_cp1, float r_cp2, float *r, flo
 	}
 }
 
+/* Returns true if the button is for a color with gamma baked in,
+ * or if it's a color picker for such a button. */
+bool ui_but_is_color_gamma(uiBut *but)
+{
+	if (but->rnaprop) {
+		if (RNA_property_subtype(but->rnaprop) == PROP_COLOR_GAMMA) {
+			return true;
+		}
+	}
+
+	return but->block->is_color_gamma_picker;
+}
+
+void ui_scene_linear_to_color_picker_space(uiBut *but, float rgb[3])
+{
+	/* Map to color picking space for HSV values and HSV cube/circle,
+	 * assuming it is more perceptually linear then the scene linear
+	 * space for intuitive color picking. */
+	if (!ui_but_is_color_gamma(but)) {
+		IMB_colormanagement_scene_linear_to_color_picking_v3(rgb);
+	}
+}
+
+void ui_color_picker_to_scene_linear_space(uiBut *but, float rgb[3])
+{
+	if (!ui_but_is_color_gamma(but)) {
+		IMB_colormanagement_color_picking_to_scene_linear_v3(rgb);
+	}
+}
+
 /** \} */
 
 /* -------------------------------------------------------------------- */
@@ -132,28 +162,18 @@ void ui_but_hsv_set(uiBut *but)
 /* Updates all buttons who share the same color picker as the one passed
  * also used by small picker, be careful with name checks below... */
 static void ui_update_color_picker_buts_rgb(
-        uiBlock *block, ColorPicker *cpicker, const float rgb[3], bool is_display_space)
+        uiBut *from_but, uiBlock *block, ColorPicker *cpicker, const float rgb[3])
 {
 	uiBut *bt;
 	float *hsv = cpicker->color_data;
-	struct ColorManagedDisplay *display = NULL;
-	/* this is to keep the H and S value when V is equal to zero
-	 * and we are working in HSV mode, of course!
-	 */
-	if (is_display_space) {
-		ui_rgb_to_color_picker_compat_v(rgb, hsv);
-	}
-	else {
-		/* we need to convert to display space to use hsv, because hsv is stored in display space */
-		float rgb_display[3];
 
-		copy_v3_v3(rgb_display, rgb);
-		ui_block_cm_to_display_space_v3(block, rgb_display);
-		ui_rgb_to_color_picker_compat_v(rgb_display, hsv);
+	/* Convert from RGB to HSV in perceptually linear space. */
+	float tmp[3];
+	copy_v3_v3(tmp, rgb);
+	if (from_but) {
+		ui_scene_linear_to_color_picker_space(from_but, tmp);
 	}
-
-	if (block->color_profile)
-		display = ui_block_cm_display_get(block);
+	ui_rgb_to_color_picker_compat_v(tmp, hsv);
 
 	/* this updates button strings, is hackish... but button pointers are on stack of caller function */
 	for (bt = block->buttons.first; bt; bt = bt->next) {
@@ -177,9 +197,9 @@ static void ui_update_color_picker_buts_rgb(
 
 			copy_v3_v3(rgb_gamma, rgb);
 
-			if (display) {
+			if (!block->is_color_gamma_picker) {
 				/* make a display version, for Hex code */
-				IMB_colormanagement_scene_linear_to_display_v3(rgb_gamma, display);
+				ui_block_cm_to_display_space_v3(block, rgb_gamma);
 			}
 
 			if (rgb_gamma[0] > 1.0f) rgb_gamma[0] = modf(rgb_gamma[0], &intpart);
@@ -230,7 +250,7 @@ static void ui_colorpicker_rna_cb(bContext *UNUSED(C), void *bt1, void *UNUSED(a
 	if (prop) {
 		RNA_property_float_get_array(&ptr, prop, rgb);
 		ui_update_color_picker_buts_rgb(
-		        but->block, but->custom_data, rgb, (RNA_property_subtype(prop) == PROP_COLOR_GAMMA));
+		        but, but->block, but->custom_data, rgb);
 	}
 
 	if (popup)
@@ -244,16 +264,13 @@ static void ui_color_wheel_rna_cb(bContext *UNUSED(C), void *bt1, void *UNUSED(a
 	float rgb[3];
 	ColorPicker *cpicker = but->custom_data;
 	float *hsv = cpicker->color_data;
-	bool use_display_colorspace = ui_but_is_colorpicker_display_space(but);
 
 	ui_color_picker_to_rgb_v(hsv, rgb);
 
-	/* hsv is saved in display space so convert back */
-	if (use_display_colorspace) {
-		ui_block_cm_to_scene_linear_v3(but->block, rgb);
-	}
+	/* hsv is saved in perceptually linear space so convert back */
+	ui_color_picker_to_scene_linear_space(but, rgb);
 
-	ui_update_color_picker_buts_rgb(but->block, cpicker, rgb, !use_display_colorspace);
+	ui_update_color_picker_buts_rgb(but, but->block, cpicker, rgb);
 
 	if (popup)
 		popup->menuretval = UI_RETURN_UPDATE;
@@ -270,12 +287,12 @@ static void ui_colorpicker_hex_rna_cb(bContext *UNUSED(C), void *bt1, void *hexc
 	hex_to_rgb(hexcol, rgb, rgb + 1, rgb + 2);
 
 	/* Hex code is assumed to be in sRGB space (coming from other applications, web, etc) */
-	if (but->block->color_profile) {
+	if (!but->block->is_color_gamma_picker) {
 		/* so we need to linearise it for Blender */
 		ui_block_cm_to_scene_linear_v3(but->block, rgb);
 	}
 
-	ui_update_color_picker_buts_rgb(but->block, cpicker, rgb, false);
+	ui_update_color_picker_buts_rgb(NULL, but->block, cpicker, rgb);
 
 	if (popup)
 		popup->menuretval = UI_RETURN_UPDATE;
@@ -378,37 +395,22 @@ static void ui_colorpicker_square(uiBlock *block, PointerRNA *ptr, PropertyRNA *
 
 /* a HS circle, V slider, rgb/hsv/hex sliders */
 static void ui_block_colorpicker(
-        uiBlock *block, float rgba[4], PointerRNA *ptr, PropertyRNA *prop, bool show_picker)
+        uiBlock *block, uiBut *from_but, float rgba[4], bool show_picker)
 {
 	static short colormode = 0;  /* temp? 0=rgb, 1=hsv, 2=hex */
 	uiBut *bt;
 	int width, butwidth;
-	static char tip[50];
 	static char hexcol[128];
-	float rgb_gamma[3];
-	unsigned char rgb_gamma_uchar[3];
 	float softmin, softmax, hardmin, hardmax, step, precision;
 	int yco;
 	ColorPicker *cpicker = ui_block_colorpicker_create(block);
 	float *hsv = cpicker->color_data;
+	PointerRNA *ptr = &from_but->rnapoin;
+	PropertyRNA *prop = from_but->rnaprop;
 
 	width = PICKER_TOTAL_W;
 	butwidth = width - 1.5f * UI_UNIT_X;
 
-	/* existence of profile means storage is in linear color space, with display correction */
-	/* XXX That tip message is not use anywhere! */
-	if (!block->color_profile) {
-		BLI_strncpy(tip, N_("Value in Display Color Space"), sizeof(tip));
-		copy_v3_v3(rgb_gamma, rgba);
-	}
-	else {
-		BLI_strncpy(tip, N_("Value in Linear RGB Color Space"), sizeof(tip));
-
-		/* make a display version, for Hex code */
-		copy_v3_v3(rgb_gamma, rgba);
-		ui_block_cm_to_display_space_v3(block, rgb_gamma);
-	}
-
 	/* sneaky way to check for alpha */
 	rgba[3] = FLT_MAX;
 
@@ -416,6 +418,11 @@ static void ui_block_colorpicker(
 	RNA_property_float_range(ptr, prop, &hardmin, &hardmax);
 	RNA_property_float_get_array(ptr, prop, rgba);
 
+	float rgb_perceptual[3];
+	copy_v3_v3(rgb_perceptual, rgba);
+	ui_scene_linear_to_color_picker_space(from_but, rgb_perceptual);
+	ui_rgb_to_color_picker_v(rgb_perceptual, hsv);
+
 	/* when the softmax isn't defined in the RNA,
 	 * using very large numbers causes sRGB/linear round trip to fail. */
 	if (softmax == FLT_MAX) {
@@ -539,6 +546,19 @@ static void ui_block_colorpicker(
 		rgba[3] = 1.0f;
 	}
 
+	/* Hex color is in display space. This should actually be sRGB without any view
+	 * transform, as most other applications would expect this. */
+	float rgb_gamma[3];
+	unsigned char rgb_gamma_uchar[3];
+
+	if (block->is_color_gamma_picker) {
+		copy_v3_v3(rgb_gamma, rgba);
+	}
+	else {
+		copy_v3_v3(rgb_gamma, rgba);
+		ui_block_cm_to_display_space_v3(block, rgb_gamma);
+	}
+
 	rgb_float_to_uchar(rgb_gamma_uchar, rgb_gamma);
 	BLI_snprintf(hexcol, sizeof(hexcol), "%02X%02X%02X", UNPACK3_EX((uint), rgb_gamma_uchar, ));
 
@@ -552,8 +572,6 @@ static void ui_block_colorpicker(
 	        block, UI_BTYPE_LABEL, 0, IFACE_("(Gamma Corrected)"), 0, yco - UI_UNIT_Y,
 	        butwidth, UI_UNIT_Y, NULL, 0.0, 0.0, 0, 0, "");
 
-	ui_rgb_to_color_picker_v(rgb_gamma, hsv);
-
 	ui_colorpicker_hide_reveal(block, colormode);
 }
 
@@ -576,24 +594,18 @@ static int ui_colorpicker_small_wheel_cb(const bContext *UNUSED(C), uiBlock *blo
 				float rgb[3];
 				ColorPicker *cpicker = but->custom_data;
 				float *hsv = cpicker->color_data;
-				bool use_display_colorspace = ui_but_is_colorpicker_display_space(but);
 
 				ui_but_v3_get(but, rgb);
-
-				if (use_display_colorspace)
-					ui_block_cm_to_display_space_v3(block, rgb);
-
+				ui_scene_linear_to_color_picker_space(but, rgb);
 				ui_rgb_to_color_picker_compat_v(rgb, hsv);
 
 				hsv[2] = clamp_f(hsv[2] + add, 0.0f, 1.0f);
-				ui_color_picker_to_rgb_v(hsv, rgb);
-
-				if (use_display_colorspace)
-					ui_block_cm_to_scene_linear_v3(block, rgb);
 
+				ui_color_picker_to_rgb_v(hsv, rgb);
+				ui_color_picker_to_scene_linear_space(but, rgb);
 				ui_but_v3_set(but, rgb);
 
-				ui_update_color_picker_buts_rgb(block, cpicker, rgb, !use_display_colorspace);
+				ui_update_color_picker_buts_rgb(but, block, cpicker, rgb);
 				if (popup)
 					popup->menuretval = UI_RETURN_UPDATE;
 
@@ -612,8 +624,8 @@ uiBlock *ui_block_func_COLOR(bContext *C, uiPopupBlockHandle *handle, void *arg_
 
 	block = UI_block_begin(C, handle->region, __func__, UI_EMBOSS);
 
-	if (RNA_property_subtype(but->rnaprop) == PROP_COLOR_GAMMA) {
-		block->color_profile = false;
+	if (ui_but_is_color_gamma(but)) {
+		block->is_color_gamma_picker = true;
 	}
 
 	if (but->block) {
@@ -626,7 +638,7 @@ uiBlock *ui_block_func_COLOR(bContext *C, uiPopupBlockHandle *handle, void *arg_
 
 	copy_v3_v3(handle->retvec, but->editvec);
 
-	ui_block_colorpicker(block, handle->retvec, &but->rnapoin, but->rnaprop, show_picker);
+	ui_block_colorpicker(block, but, handle->retvec, show_picker);
 
 	block->flag = UI_BLOCK_LOOP | UI_BLOCK_KEEP_OPEN | UI_BLOCK_OUT_1 | UI_BLOCK_MOVEMOUSE_QUIT;
 	UI_block_theme_style_set(block, UI_BLOCK_THEME_STYLE_POPUP);