[Bf-blender-cvs] [6601a89650f] blender2.8: Fix T58549, T56741: HSV color picker issues with Filmic view transform.

Brecht Van Lommel noreply at git.blender.org
Thu Dec 13 19:31:55 CET 2018


Commit: 6601a89650f92454aa57bc01bedebd4086f6d98d
Author: Brecht Van Lommel
Date:   Thu Dec 13 15:59:58 2018 +0100
Branches: blender2.8
https://developer.blender.org/rB6601a89650f92454aa57bc01bedebd4086f6d98d

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.

===================================================================

M	release/datafiles/colormanagement/config.ocio
M	source/blender/editors/interface/interface.c
M	source/blender/editors/interface/interface_draw.c
M	source/blender/editors/interface/interface_handlers.c
M	source/blender/editors/interface/interface_intern.h
M	source/blender/editors/interface/interface_region_color_picker.c
M	source/blender/editors/interface/interface_widgets.c
M	source/blender/imbuf/IMB_colormanagement.h
M	source/blender/imbuf/intern/colormanagement.c

===================================================================

diff --git a/release/datafiles/colormanagement/config.ocio b/release/datafiles/colormanagement/config.ocio
index bba5958769d..411af8ebdf2 100644
--- a/release/datafiles/colormanagement/config.ocio
+++ b/release/datafiles/colormanagement/config.ocio
@@ -34,7 +34,7 @@ roles:
   default_sequencer: sRGB
 
   # Color spaces for color picking and texture painting (not internally supported yet)
-  color_picking: Raw
+  color_picking: sRGB
   texture_paint: Raw
 
   # Non-color data
diff --git a/source/blender/editors/interface/interface.c b/source/blender/editors/interface/interface.c
index 39dcdda5ee8..2a4dfc210b0 100644
--- a/source/blender/editors/interface/interface.c
+++ b/source/blender/editors/interface/interface.c
@@ -2992,19 +2992,20 @@ uiBlock *UI_block_begin(const bContext *C, ARegion *region, const char *name, sh
 	block->evil_C = (void *)C;  /* XXX */
 
 	if (scn) {
-		block->color_profile = true;
-
 		/* store display device name, don't lookup for transformations yet
 		 * block could be used for non-color displays where looking up for transformation
 		 * would slow down redraw, so only lookup for actual transform when it's indeed
 		 * needed
 		 */
-		BLI_strncpy(block->display_device, scn->display_settings.display_device, sizeof(block->display_device));
+		STRNCPY(block->display_device, scn->display_settings.display_device);
 
 		/* copy to avoid crash when scene gets deleted with ui still open */
 		block->unit = MEM_mallocN(sizeof(scn->unit), "UI UnitSettings");
 		memcpy(block->unit, &scn->unit, sizeof(scn->unit));
 	}
+	else {
+		STRNCPY(block->display_device, IMB_colormanagement_display_get_default_name());
+	}
 
 	BLI_strncpy(block->name, name, sizeof(block->name));
 
@@ -3295,20 +3296,6 @@ void ui_block_cm_to_scene_linear_v3(uiBlock *block, float pixel[3])
 	IMB_colormanagement_display_to_scene_linear_v3(pixel, display);
 }
 
-void ui_block_cm_to_display_space_range(uiBlock *block, float *min, float *max)
-{
-	struct ColorManagedDisplay *display = ui_block_cm_display_get(block);
-	float pixel[3];
-
-	copy_v3_fl(pixel, *min);
-	IMB_colormanagement_scene_linear_to_display_v3(pixel, display);
-	*min = min_fff(UNPACK3(pixel));
-
-	copy_v3_fl(pixel, *max);
-	IMB_colormanagement_scene_linear_to_display_v3(pixel, display);
-	*max = max_fff(UNPACK3(pixel));
-}
-
 static uiBut *ui_but_alloc(const eButType type)
 {
 	switch (type) {
diff --git a/source/blender/editors/interface/interface_draw.c b/source/blender/editors/interface/interface_draw.c
index ef9a69cd1dd..f1ada343faa 100644
--- a/source/blender/editors/interface/interface_draw.c
+++ b/source/blender/editors/interface/interface_draw.c
@@ -1404,15 +1404,12 @@ static void ui_draw_colorband_handle(
 
 void ui_draw_but_COLORBAND(uiBut *but, uiWidgetColors *UNUSED(wcol), const rcti *rect)
 {
-	struct ColorManagedDisplay *display = NULL;
+	struct ColorManagedDisplay *display = ui_block_cm_display_get(but->block);
 	uint pos_id, col_id;
 
 	ColorBand *coba = (ColorBand *)(but->editcoba ? but->editcoba : but->poin);
 	if (coba == NULL) return;
 
-	if (but->block->color_profile)
-		display = ui_block_cm_display_get(but->block);
-
 	float x1 = rect->xmin;
 	float sizex = rect->xmax - x1;
 	float sizey = BLI_rcti_size_y(rect);
diff --git a/source/blender/editors/interface/interface_handlers.c b/source/blender/editors/interface/interface_handlers.c
index 3a3259e8d1f..3949c1e2d50 100644
--- a/source/blender/editors/interface/interface_handlers.c
+++ b/source/blender/editors/interface/interface_handlers.c
@@ -5440,7 +5440,6 @@ static bool ui_numedit_but_HSVCUBE(
 	float x, y;
 	float mx_fl, my_fl;
 	bool changed = true;
-	bool use_display_colorspace = ui_but_is_colorpicker_display_space(but);
 
 	ui_mouse_scale_warp(data, mx, my, &mx_fl, &my_fl, shift);
 
@@ -5454,9 +5453,7 @@ static bool ui_numedit_but_HSVCUBE(
 #endif
 
 	ui_but_v3_get(but, rgb);
-
-	if (use_display_colorspace)
-		ui_block_cm_to_display_space_v3(but->block, rgb);
+	ui_scene_linear_to_color_picker_space(but, rgb);
 
 	ui_rgb_to_color_picker_HSVCUBE_compat_v(but, rgb, hsv);
 
@@ -5469,8 +5466,7 @@ static bool ui_numedit_but_HSVCUBE(
 
 		/* calculate original hsv again */
 		copy_v3_v3(rgb, data->origvec);
-		if (use_display_colorspace)
-			ui_block_cm_to_display_space_v3(but->block, rgb);
+		ui_scene_linear_to_color_picker_space(but, rgb);
 
 		copy_v3_v3(hsvo, hsv);
 
@@ -5518,9 +5514,6 @@ static bool ui_numedit_but_HSVCUBE(
 		{
 			/* vertical 'value' strip */
 			float min = but->softmin, max = but->softmax;
-			if (use_display_colorspace) {
-				ui_block_cm_to_display_space_range(but->block, &min, &max);
-			}
 			/* exception only for value strip - use the range set in but->min/max */
 			hsv[2] = y * (max - min) + min;
 			break;
@@ -5537,9 +5530,7 @@ static bool ui_numedit_but_HSVCUBE(
 	}
 
 	ui_color_picker_to_rgb_HSVCUBE_v(but, hsv, rgb);
-
-	if (use_display_colorspace)
-		ui_block_cm_to_scene_linear_v3(but->block, rgb);
+	ui_color_picker_to_scene_linear_space(but, rgb);
 
 	/* clamp because with color conversion we can exceed range [#34295] */
 	if (but->a1 == UI_GRAD_V_ALT) {
@@ -5565,13 +5556,9 @@ static void ui_ndofedit_but_HSVCUBE(
 	const float hsv_v_max = max_ff(hsv[2], but->softmax);
 	float rgb[3];
 	float sensitivity = (shift ? 0.15f : 0.3f) * ndof->dt;
-	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(but->block, rgb);
-
+	ui_scene_linear_to_color_picker_space(but, rgb);
 	ui_rgb_to_color_picker_HSVCUBE_compat_v(but, rgb, hsv);
 
 	switch ((int)but->a1) {
@@ -5620,9 +5607,7 @@ static void ui_ndofedit_but_HSVCUBE(
 	hsv_clamp_v(hsv, hsv_v_max);
 
 	ui_color_picker_to_rgb_HSVCUBE_v(but, hsv, rgb);
-
-	if (use_display_colorspace)
-		ui_block_cm_to_scene_linear_v3(but->block, rgb);
+	ui_color_picker_to_scene_linear_space(but, rgb);
 
 	copy_v3_v3(data->vec, rgb);
 	ui_but_v3_set(but, data->vec);
@@ -5737,7 +5722,6 @@ static bool ui_numedit_but_HSVCIRCLE(
 	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_mouse_scale_warp(data, mx, my, &mx_fl, &my_fl, shift);
 
@@ -5760,9 +5744,7 @@ static bool ui_numedit_but_HSVCIRCLE(
 	BLI_rcti_rctf_copy(&rect, &but->rect);
 
 	ui_but_v3_get(but, rgb);
-	if (use_display_colorspace)
-		ui_block_cm_to_display_space_v3(but->block, rgb);
-
+	ui_scene_linear_to_color_picker_space(but, rgb);
 	ui_rgb_to_color_picker_compat_v(rgb, hsv);
 
 	/* exception, when using color wheel in 'locked' value state:
@@ -5784,9 +5766,7 @@ static bool ui_numedit_but_HSVCIRCLE(
 		/* calculate original hsv again */
 		copy_v3_v3(hsvo, hsv);
 		copy_v3_v3(rgbo, data->origvec);
-		if (use_display_colorspace)
-			ui_block_cm_to_display_space_v3(but->block, rgbo);
-
+		ui_scene_linear_to_color_picker_space(but, rgbo);
 		ui_rgb_to_color_picker_compat_v(rgbo, hsvo);
 
 		/* and original position */
@@ -5812,9 +5792,7 @@ static bool ui_numedit_but_HSVCIRCLE(
 		normalize_v3_length(rgb, but->a2);
 	}
 
-	if (use_display_colorspace)
-		ui_block_cm_to_scene_linear_v3(but->block, rgb);
-
+	ui_color_picker_to_scene_linear_space(but, rgb);
 	ui_but_v3_set(but, rgb);
 
 	data->draglastx = mx;
@@ -5831,14 +5809,12 @@ static void ui_ndofedit_but_HSVCIRCLE(
 {
 	ColorPicker *cpicker = but->custom_data;
 	float *hsv = cpicker->color_data;
-	bool use_display_colorspace = ui_but_is_colorpicker_display_space(but);
 	float rgb[3];
 	float phi, r /*, sqr */ /* UNUSED */, v[2];
 	float sensitivity = (shift ? 0.06f : 0.3f) * ndof->dt;
 
 	ui_but_v3_get(but, rgb);
-	if (use_display_colorspace)
-		ui_block_cm_to_display_space_v3(but->block, rgb);
+	ui_scene_linear_to_color_picker_space(but, rgb);
 	ui_rgb_to_color_picker_compat_v(rgb, hsv);
 
 	/* Convert current color on hue/sat disc to circular coordinates phi, r */
@@ -5889,9 +5865,7 @@ static void ui_ndofedit_but_HSVCIRCLE(
 		normalize_v3_length(data->vec, but->a2);
 	}
 
-	if (use_display_colorspace)
-		ui_block_cm_to_scene_linear_v3(but->block, data->vec);
-
+	ui_color_picker_to_scene_linear_space(but, data->vec);
 	ui_but_v3_set(but, data->vec);
 }
 #endif /* WITH_INPUT_NDOF */
diff --git a/source/blender/editors/interface/interface_intern.h b/source/blender/editors/interface/interface_intern.h
index 7e8653bfc56..0ab92a633ad 100644
--- a/source/blender/editors/interface/interface_intern.h
+++ b/source/blender/editors/interface/interface_intern.h
@@ -439,7 +439,7 @@ struct uiBlock {
 	struct UnitSettings *unit;  /* unit system, used a lot for numeric buttons so include here rather then fetching through the scene every time. */
 	ColorPickerData color_pickers; /* XXX, only accessed by color picker templates */
 
-	bool color_profile;         /* color profile for correcting linear colors for display */
+	bool is_color_gamma_picker; /* Block for color picker with gamma baked in. */
 
 	char display_device[64]; /* display device name used to display this block,
 	                          * used by color widgets to transform colors from/to scene linear
@@ -477,7 +477,6 @@ extern void ui_hsvcircle_vals_from_pos(
         const float mx, const float my);
 extern void ui_hsvcircle_pos_from_vals(struct uiBut *but, const rcti *rect, float *hsv, float *xpos, float *ypos);
 extern void ui_hsvcube_pos_from_vals(struct uiBut *but, const rcti *rect, float *hsv, float *xp, float *yp);
-bool ui_but_is_colorpicker_display_space(struct uiBut *but);
 
 extern void ui_but_string_get_ex(
         uiBut *but, char *str, const size_t maxlen,
@@ -516,7 +515,6 @@ extern void ui_block_bounds_calc(uiBlock *block);
 extern struct ColorManagedDisplay *ui_block_cm_display_get(uiBlock *block);
 void ui_block_cm_to_display_space_v3(uiBlock *block, float pixel[3]);
 void ui_block_cm_to_scene_linear_v3(uiBlock *block, float pixel[3]);
-void ui_block_cm_to_display_space_range(uiBlock *block, float *min, float *max);
 
 /* interface_regions.c */
 
@@ -611,6 +609,11 @@ void ui_rgb_to_color_picker_v(const float rgb[3], float r_cp[3]);
 void ui_color_picker_to_rgb_v(const float r_cp[3], float rgb[3]);
 void ui_color_picker_to_rgb(float r_cp0, float r_cp1, float r

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list