[Bf-blender-cvs] [f193b1afb31] master: Color Management: use scene linear HSV for number buttons in color pickers

Brecht Van Lommel noreply at git.blender.org
Thu Dec 17 20:12:49 CET 2020


Commit: f193b1afb313bcb937a396f09da2b9903a0d2fc3
Author: Brecht Van Lommel
Date:   Thu Dec 17 19:29:39 2020 +0100
Branches: master
https://developer.blender.org/rBf193b1afb313bcb937a396f09da2b9903a0d2fc3

Color Management: use scene linear HSV for number buttons in color pickers

Previously these HSV values were in the color picking space, which meant the
relation to the scene linear RGB values was confusing.

The new situation:
* RGB number buttons: scene linear color space
* HSV number buttons: scene linear color space
* Picker widgets: color picking color space
* Hex: sRGB color space

Fixes T69562, T83853, Ref T68926

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

M	source/blender/editors/interface/interface_intern.h
M	source/blender/editors/interface/interface_region_color_picker.c

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

diff --git a/source/blender/editors/interface/interface_intern.h b/source/blender/editors/interface/interface_intern.h
index 5c7cad4c8d5..8f894d0c4bb 100644
--- a/source/blender/editors/interface/interface_intern.h
+++ b/source/blender/editors/interface/interface_intern.h
@@ -378,11 +378,20 @@ typedef struct uiButExtraOpIcon {
 
 typedef struct ColorPicker {
   struct ColorPicker *next, *prev;
-  /** Color data, may be HSV or HSL. */
+
+  /** Color in HSV or HSL, in color picking color space. Used for HSV cube,
+   * circle and slider widgets. The color picking space is perceptually
+   * linear for intuitive editing. */
   float color_data[3];
-  /** Initial color data (detect changes). */
+  /** Initial color data (to detect changes). */
   float color_data_init[3];
   bool is_init;
+
+  /** HSV or HSL color in scene linear color space value used for number
+   * buttons. This is scene linear so that there is a clear correspondence
+   * to the scene linear RGB values. */
+  float hsv[3];
+
   /** Cubic saturation for the color wheel. */
   bool use_color_cubic;
   bool use_color_lock;
diff --git a/source/blender/editors/interface/interface_region_color_picker.c b/source/blender/editors/interface/interface_region_color_picker.c
index f9a9e7182d2..cbf3dbf393d 100644
--- a/source/blender/editors/interface/interface_region_color_picker.c
+++ b/source/blender/editors/interface/interface_region_color_picker.c
@@ -182,15 +182,16 @@ static void ui_update_color_picker_buts_rgb(uiBut *from_but,
                                             ColorPicker *cpicker,
                                             const float rgb[3])
 {
-  float *hsv = cpicker->color_data;
+  /* Convert from RGB to HSV in scene linear space color for numeric editing. */
+  ui_rgb_to_color_picker_compat_v(rgb, cpicker->hsv);
 
-  /* Convert from RGB to HSV in perceptually linear space. */
+  /* Convert from RGB to HSV in perceptually linear space for picker widgets. */
   float tmp[3];
   copy_v3_v3(tmp, rgb);
   if (from_but) {
     ui_scene_linear_to_color_picker_space(from_but, tmp);
   }
-  ui_rgb_to_color_picker_compat_v(tmp, hsv);
+  ui_rgb_to_color_picker_compat_v(tmp, cpicker->color_data);
 
   /* this updates button strings,
    * is hackish... but button pointers are on stack of caller function */
@@ -235,16 +236,16 @@ static void ui_update_color_picker_buts_rgb(uiBut *from_but,
         ui_but_value_set(bt, rgb[2]);
       }
       else if (bt->str[0] == 'H') {
-        ui_but_value_set(bt, hsv[0]);
+        ui_but_value_set(bt, cpicker->hsv[0]);
       }
       else if (bt->str[0] == 'S') {
-        ui_but_value_set(bt, hsv[1]);
+        ui_but_value_set(bt, cpicker->hsv[1]);
       }
       else if (bt->str[0] == 'V') {
-        ui_but_value_set(bt, hsv[2]);
+        ui_but_value_set(bt, cpicker->hsv[2]);
       }
       else if (bt->str[0] == 'L') {
-        ui_but_value_set(bt, hsv[2]);
+        ui_but_value_set(bt, cpicker->hsv[2]);
       }
     }
 
@@ -252,7 +253,7 @@ static void ui_update_color_picker_buts_rgb(uiBut *from_but,
   }
 }
 
-static void ui_colorpicker_rna_cb(bContext *UNUSED(C), void *bt1, void *UNUSED(arg))
+static void ui_colorpicker_rgba_update_cb(bContext *UNUSED(C), void *bt1, void *UNUSED(arg))
 {
   uiBut *but = (uiBut *)bt1;
   uiPopupBlockHandle *popup = but->block->handle;
@@ -270,19 +271,14 @@ static void ui_colorpicker_rna_cb(bContext *UNUSED(C), void *bt1, void *UNUSED(a
   }
 }
 
-static void ui_color_wheel_rna_cb(bContext *UNUSED(C), void *bt1, void *UNUSED(arg))
+static void ui_colorpicker_hsv_update_cb(bContext *UNUSED(C), void *bt1, void *UNUSED(arg))
 {
   uiBut *but = (uiBut *)bt1;
   uiPopupBlockHandle *popup = but->block->handle;
   float rgb[3];
   ColorPicker *cpicker = but->custom_data;
-  float *hsv = cpicker->color_data;
-
-  ui_color_picker_to_rgb_v(hsv, rgb);
-
-  /* hsv is saved in perceptually linear space so convert back */
-  ui_color_picker_to_scene_linear_space(but, rgb);
 
+  ui_color_picker_to_rgb_v(cpicker->hsv, rgb);
   ui_update_color_picker_buts_rgb(but, but->block, cpicker, rgb);
 
   if (popup) {
@@ -331,12 +327,12 @@ static void ui_colorpicker_hide_reveal(uiBlock *block, enum ePickerType colormod
 {
   /* tag buttons */
   LISTBASE_FOREACH (uiBut *, bt, &block->buttons) {
-    if ((bt->func == ui_colorpicker_rna_cb) && (bt->type == UI_BTYPE_NUM_SLIDER) &&
+    if ((bt->func == ui_colorpicker_rgba_update_cb) && (bt->type == UI_BTYPE_NUM_SLIDER) &&
         (bt->rnaindex != 3)) {
       /* RGB sliders (color circle and alpha are always shown) */
       SET_FLAG_FROM_TEST(bt->flag, (colormode != PICKER_TYPE_RGB), UI_HIDDEN);
     }
-    else if (bt->func == ui_color_wheel_rna_cb) {
+    else if (bt->func == ui_colorpicker_hsv_update_cb) {
       /* HSV sliders */
       SET_FLAG_FROM_TEST(bt->flag, (colormode != PICKER_TYPE_HSV), UI_HIDDEN);
     }
@@ -386,7 +382,7 @@ static void ui_colorpicker_circle(uiBlock *block,
                       0.0,
                       0,
                       TIP_("Color"));
-  UI_but_func_set(bt, ui_colorpicker_rna_cb, bt, NULL);
+  UI_but_func_set(bt, ui_colorpicker_rgba_update_cb, bt, NULL);
   bt->custom_data = cpicker;
 
   /* value */
@@ -408,7 +404,7 @@ static void ui_colorpicker_circle(uiBlock *block,
                                              0,
                                              "Lightness");
     hsv_but->gradient_type = UI_GRAD_L_ALT;
-    UI_but_func_set(&hsv_but->but, ui_colorpicker_rna_cb, &hsv_but->but, NULL);
+    UI_but_func_set(&hsv_but->but, ui_colorpicker_rgba_update_cb, &hsv_but->but, NULL);
   }
   else {
     hsv_but = (uiButHSVCube *)uiDefButR_prop(block,
@@ -428,7 +424,7 @@ static void ui_colorpicker_circle(uiBlock *block,
                                              0,
                                              TIP_("Value"));
     hsv_but->gradient_type = UI_GRAD_V_ALT;
-    UI_but_func_set(&hsv_but->but, ui_colorpicker_rna_cb, &hsv_but->but, NULL);
+    UI_but_func_set(&hsv_but->but, ui_colorpicker_rgba_update_cb, &hsv_but->but, NULL);
   }
   hsv_but->but.custom_data = cpicker;
 }
@@ -461,7 +457,7 @@ static void ui_colorpicker_square(uiBlock *block,
                                            0,
                                            TIP_("Color"));
   hsv_but->gradient_type = type;
-  UI_but_func_set(&hsv_but->but, ui_colorpicker_rna_cb, &hsv_but->but, NULL);
+  UI_but_func_set(&hsv_but->but, ui_colorpicker_rgba_update_cb, &hsv_but->but, NULL);
   hsv_but->but.custom_data = cpicker;
 
   /* value */
@@ -482,7 +478,7 @@ static void ui_colorpicker_square(uiBlock *block,
                                            0,
                                            TIP_("Value"));
   hsv_but->gradient_type = type + 3;
-  UI_but_func_set(&hsv_but->but, ui_colorpicker_rna_cb, &hsv_but->but, NULL);
+  UI_but_func_set(&hsv_but->but, ui_colorpicker_rgba_update_cb, &hsv_but->but, NULL);
   hsv_but->but.custom_data = cpicker;
 }
 
@@ -497,7 +493,6 @@ static void ui_block_colorpicker(uiBlock *block, uiBut *from_but, float rgba[4],
   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;
 
@@ -513,8 +508,9 @@ static void ui_block_colorpicker(uiBlock *block, uiBut *from_but, float rgba[4],
 
   float rgb_perceptual[3];
   copy_v3_v3(rgb_perceptual, rgba);
+  ui_rgb_to_color_picker_v(rgba, cpicker->hsv);
   ui_scene_linear_to_color_picker_space(from_but, rgb_perceptual);
-  ui_rgb_to_color_picker_v(rgb_perceptual, hsv);
+  ui_rgb_to_color_picker_v(rgb_perceptual, cpicker->color_data);
   if (cpicker->is_init == false) {
     copy_v3_v3(cpicker->color_data_init, cpicker->color_data);
     cpicker->is_init = true;
@@ -639,7 +635,7 @@ static void ui_block_colorpicker(uiBlock *block, uiBut *from_but, float rgba[4],
                       0,
                       3,
                       TIP_("Red"));
-  UI_but_func_set(bt, ui_colorpicker_rna_cb, bt, NULL);
+  UI_but_func_set(bt, ui_colorpicker_rgba_update_cb, bt, NULL);
   bt->custom_data = cpicker;
   bt = uiDefButR_prop(block,
                       UI_BTYPE_NUM_SLIDER,
@@ -657,7 +653,7 @@ static void ui_block_colorpicker(uiBlock *block, uiBut *from_but, float rgba[4],
                       0,
                       3,
                       TIP_("Green"));
-  UI_but_func_set(bt, ui_colorpicker_rna_cb, bt, NULL);
+  UI_but_func_set(bt, ui_colorpicker_rgba_update_cb, bt, NULL);
   bt->custom_data = cpicker;
   bt = uiDefButR_prop(block,
                       UI_BTYPE_NUM_SLIDER,
@@ -675,7 +671,7 @@ static void ui_block_colorpicker(uiBlock *block, uiBut *from_but, float rgba[4],
                       0,
                       3,
                       TIP_("Blue"));
-  UI_but_func_set(bt, ui_colorpicker_rna_cb, bt, NULL);
+  UI_but_func_set(bt, ui_colorpicker_rgba_update_cb, bt, NULL);
   bt->custom_data = cpicker;
 
   /* Could use:
@@ -693,14 +689,14 @@ static void ui_block_colorpicker(uiBlock *block, uiBut *from_but, float rgba[4],
                  yco,
                  butwidth,
                  UI_UNIT_Y,
-                 hsv,
+                 cpicker->hsv,
                  0.0,
                  1.0,
                  10,
                  3,
                  TIP_("Hue"));
   UI_but_flag_disable(bt, UI_BUT_UNDO);
-  UI_but_func_set(bt, ui_color_wheel_rna_cb, bt, hsv);
+  UI_but_func_set(bt, ui_colorpicker_hsv_update_cb, bt, NULL);
   bt->custom_data = cpicker;
   bt = uiDefButF(block,
                  UI_BTYPE_NUM_SLIDER,
@@ -710,14 +706,14 @@ static void ui_block_colorpicker(uiBlock *block, uiBut *from_but, float rgba[4],
                  yco -= UI_UNIT_Y,
                  butwidth,
                  UI_UNIT_Y,
-                 hsv + 1,
+                 cpicker->hsv + 1,
                  0.0,
                  1.0,
                  10,
                  3,
                  TIP_("Saturation"));
   UI_but_flag_disable(bt, UI_BUT_UNDO);
-  UI_but_func_set(bt, ui_color_wheel_rna_cb, bt, hsv);
+  UI_but_func_set(bt, ui_colorpicker_hsv_update_cb, bt, NULL);
   bt->custom_data =

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list