Cleanup: renaming and code deduplication for color space clarity in picker

Ref T68926

1 files changed, 84 insertions, 77 deletions

diff --git a/source/blender/editors/interface/interface_region_color_picker.c b/source/blender/editors/interface/interface_region_color_picker.c
index cbf3dbf393d..82028d4e595 100644
--- a/source/blender/editors/interface/interface_region_color_picker.c
+++ b/source/blender/editors/interface/interface_region_color_picker.c
@@ -77,8 +77,10 @@ static void ui_color_picker_rgb_round(float rgb[3])
   }
 }
 
-void ui_rgb_to_color_picker_compat_v(const float rgb[3], float r_cp[3])
+void ui_color_picker_rgb_to_hsv_compat(const float rgb[3], float r_cp[3])
 {
+  /* Convert RGB to HSV, remaining as compatible as possible with the existing
+   * r_hsv value (for example when value goes to zero, preserve the hue). */
   switch (U.color_picker_type) {
     case USER_CP_CIRCLE_HSL:
       rgb_to_hsl_compat_v(rgb, r_cp);
@@ -89,7 +91,7 @@ void ui_rgb_to_color_picker_compat_v(const float rgb[3], float r_cp[3])
   }
 }
 
-void ui_rgb_to_color_picker_v(const float rgb[3], float r_cp[3])
+void ui_color_picker_rgb_to_hsv(const float rgb[3], float r_cp[3])
 {
   switch (U.color_picker_type) {
     case USER_CP_CIRCLE_HSL:
@@ -101,7 +103,7 @@ 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_hsv_to_rgb(const float r_cp[3], float rgb[3])
 {
   switch (U.color_picker_type) {
     case USER_CP_CIRCLE_HSL:
@@ -113,18 +115,6 @@ 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_cp2, float *r, float *g, float *b)
-{
-  switch (U.color_picker_type) {
-    case USER_CP_CIRCLE_HSL:
-      hsl_to_rgb(r_cp0, r_cp1, r_cp2, r, g, b);
-      break;
-    default:
-      hsv_to_rgb(r_cp0, r_cp1, r_cp2, r, g, b);
-      break;
-  }
-}
-
 /* 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)
@@ -138,7 +128,7 @@ bool ui_but_is_color_gamma(uiBut *but)
   return but->block->is_color_gamma_picker;
 }
 
-void ui_scene_linear_to_color_picker_space(uiBut *but, float rgb[3])
+void ui_scene_linear_to_perceptual_space(uiBut *but, float rgb[3])
 {
   /* Map to color picking space for HSV values and HSV cube/circle,
    * assuming it is more perceptually linear than the scene linear
@@ -149,7 +139,7 @@ void ui_scene_linear_to_color_picker_space(uiBut *but, float rgb[3])
   }
 }
 
-void ui_color_picker_to_scene_linear_space(uiBut *but, float rgb[3])
+void ui_perceptual_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);
@@ -163,16 +153,46 @@ void ui_color_picker_to_scene_linear_space(uiBut *but, float rgb[3])
 /** \name Color Picker
  * \{ */
 
+static void ui_color_picker_update_hsv(ColorPicker *cpicker,
+                                       uiBut *from_but,
+                                       const float rgb_scene_linear[3])
+{
+  /* Convert from RGB to HSV in scene linear space color for number editing. */
+  if (cpicker->is_init == false) {
+    ui_color_picker_rgb_to_hsv(rgb_scene_linear, cpicker->hsv_scene_linear);
+  }
+  else {
+    ui_color_picker_rgb_to_hsv_compat(rgb_scene_linear, cpicker->hsv_scene_linear);
+  }
+
+  /* Convert from RGB to HSV in perceptually linear space for picker widgets. */
+  float rgb_perceptual[3];
+  copy_v3_v3(rgb_perceptual, rgb_scene_linear);
+  if (from_but) {
+    ui_scene_linear_to_perceptual_space(from_but, rgb_perceptual);
+  }
+
+  if (cpicker->is_init == false) {
+    ui_color_picker_rgb_to_hsv(rgb_perceptual, cpicker->hsv_perceptual);
+    copy_v3_v3(cpicker->hsv_perceptual_init, cpicker->hsv_perceptual);
+  }
+  else {
+    ui_color_picker_rgb_to_hsv_compat(rgb_perceptual, cpicker->hsv_perceptual);
+  }
+
+  cpicker->is_init = true;
+}
+
 /* for picker, while editing hsv */
 void ui_but_hsv_set(uiBut *but)
 {
-  float col[3];
+  float rgb_perceptual[3];
   ColorPicker *cpicker = but->custom_data;
-  float *hsv = cpicker->color_data;
+  float *hsv_perceptual = cpicker->hsv_perceptual;
 
-  ui_color_picker_to_rgb_v(hsv, col);
+  ui_color_picker_hsv_to_rgb(hsv_perceptual, rgb_perceptual);
 
-  ui_but_v3_set(but, col);
+  ui_but_v3_set(but, rgb_perceptual);
 }
 
 /* Updates all buttons who share the same color picker as the one passed
@@ -180,18 +200,9 @@ void ui_but_hsv_set(uiBut *but)
 static void ui_update_color_picker_buts_rgb(uiBut *from_but,
                                             uiBlock *block,
                                             ColorPicker *cpicker,
-                                            const float rgb[3])
+                                            const float rgb_scene_linear[3])
 {
-  /* 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 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, cpicker->color_data);
+  ui_color_picker_update_hsv(cpicker, from_but, rgb_scene_linear);
 
   /* this updates button strings,
    * is hackish... but button pointers are on stack of caller function */
@@ -201,7 +212,7 @@ static void ui_update_color_picker_buts_rgb(uiBut *from_but,
     }
 
     if (bt->rnaprop) {
-      ui_but_v3_set(bt, rgb);
+      ui_but_v3_set(bt, rgb_scene_linear);
 
       /* original button that created the color picker already does undo
        * push, so disable it on RNA buttons in the color picker block */
@@ -214,7 +225,7 @@ static void ui_update_color_picker_buts_rgb(uiBut *from_but,
 
       /* Hex code is assumed to be in sRGB space
        * (coming from other applications, web, etc) */
-      copy_v3_v3(rgb_hex, rgb);
+      copy_v3_v3(rgb_hex, rgb_scene_linear);
       if (from_but && !ui_but_is_color_gamma(from_but)) {
         IMB_colormanagement_scene_linear_to_srgb_v3(rgb_hex);
         ui_color_picker_rgb_round(rgb_hex);
@@ -227,25 +238,25 @@ static void ui_update_color_picker_buts_rgb(uiBut *from_but,
     }
     else if (bt->str[1] == ' ') {
       if (bt->str[0] == 'R') {
-        ui_but_value_set(bt, rgb[0]);
+        ui_but_value_set(bt, rgb_scene_linear[0]);
       }
       else if (bt->str[0] == 'G') {
-        ui_but_value_set(bt, rgb[1]);
+        ui_but_value_set(bt, rgb_scene_linear[1]);
       }
       else if (bt->str[0] == 'B') {
-        ui_but_value_set(bt, rgb[2]);
+        ui_but_value_set(bt, rgb_scene_linear[2]);
       }
       else if (bt->str[0] == 'H') {
-        ui_but_value_set(bt, cpicker->hsv[0]);
+        ui_but_value_set(bt, cpicker->hsv_scene_linear[0]);
       }
       else if (bt->str[0] == 'S') {
-        ui_but_value_set(bt, cpicker->hsv[1]);
+        ui_but_value_set(bt, cpicker->hsv_scene_linear[1]);
       }
       else if (bt->str[0] == 'V') {
-        ui_but_value_set(bt, cpicker->hsv[2]);
+        ui_but_value_set(bt, cpicker->hsv_scene_linear[2]);
       }
       else if (bt->str[0] == 'L') {
-        ui_but_value_set(bt, cpicker->hsv[2]);
+        ui_but_value_set(bt, cpicker->hsv_scene_linear[2]);
       }
     }
 
@@ -259,11 +270,11 @@ static void ui_colorpicker_rgba_update_cb(bContext *UNUSED(C), void *bt1, void *
   uiPopupBlockHandle *popup = but->block->handle;
   PropertyRNA *prop = but->rnaprop;
   PointerRNA ptr = but->rnapoin;
-  float rgb[4];
+  float rgb_scene_linear[4];
 
   if (prop) {
-    RNA_property_float_get_array(&ptr, prop, rgb);
-    ui_update_color_picker_buts_rgb(but, but->block, but->custom_data, rgb);
+    RNA_property_float_get_array(&ptr, prop, rgb_scene_linear);
+    ui_update_color_picker_buts_rgb(but, but->block, but->custom_data, rgb_scene_linear);
   }
 
   if (popup) {
@@ -275,11 +286,11 @@ static void ui_colorpicker_hsv_update_cb(bContext *UNUSED(C), void *bt1, void *U
 {
   uiBut *but = (uiBut *)bt1;
   uiPopupBlockHandle *popup = but->block->handle;
-  float rgb[3];
+  float rgb_scene_linear[3];
   ColorPicker *cpicker = but->custom_data;
 
-  ui_color_picker_to_rgb_v(cpicker->hsv, rgb);
-  ui_update_color_picker_buts_rgb(but, but->block, cpicker, rgb);
+  ui_color_picker_hsv_to_rgb(cpicker->hsv_scene_linear, rgb_scene_linear);
+  ui_update_color_picker_buts_rgb(but, but->block, cpicker, rgb_scene_linear);
 
   if (popup) {
     popup->menuretval = UI_RETURN_UPDATE;
@@ -317,7 +328,7 @@ static void ui_popup_close_cb(bContext *UNUSED(C), void *bt1, void *UNUSED(arg))
   if (popup) {
     ColorPicker *cpicker = but->custom_data;
     BLI_assert(cpicker->is_init);
-    popup->menuretval = (equals_v3v3(cpicker->color_data, cpicker->color_data_init) ?
+    popup->menuretval = (equals_v3v3(cpicker->hsv_perceptual, cpicker->hsv_perceptual_init) ?
                              UI_RETURN_CANCEL :
                              UI_RETURN_OK);
   }
@@ -483,7 +494,10 @@ static void ui_colorpicker_square(uiBlock *block,
 }
 
 /* a HS circle, V slider, rgb/hsv/hex sliders */
-static void ui_block_colorpicker(uiBlock *block, uiBut *from_but, float rgba[4], bool show_picker)
+static void ui_block_colorpicker(uiBlock *block,
+                                 uiBut *from_but,
+                                 float rgba_scene_linear[4],
+                                 bool show_picker)
 {
   /* ePickerType */
   static char colormode = 1;
@@ -500,21 +514,13 @@ static void ui_block_colorpicker(uiBlock *block, uiBut *from_but, float rgba[4],
   butwidth = width - 1.5f * UI_UNIT_X;
 
   /* sneaky way to check for alpha */
-  rgba[3] = FLT_MAX;
+  rgba_scene_linear[3] = FLT_MAX;
 
   RNA_property_float_ui_range(ptr, prop, &softmin, &softmax, &step, &precision);
   RNA_property_float_range(ptr, prop, &hardmin, &hardmax);
-  RNA_property_float_get_array(ptr, prop, rgba);
+  RNA_property_float_get_array(ptr, prop, rgba_scene_linear);
 
-  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, cpicker->color_data);
-  if (cpicker->is_init == false) {
-    copy_v3_v3(cpicker->color_data_init, cpicker->color_data);
-    cpicker->is_init = true;
-  }
+  ui_color_picker_update_hsv(cpicker, from_but, rgba_scene_linear);
 
   /* when the softmax isn't defined in the RNA,
    * using very large numbers causes sRGB/linear round trip to fail. */
@@ -689,7 +695,7 @@ static void ui_block_colorpicker(uiBlock *block, uiBut *from_but, float rgba[4],
                  yco,
                  butwidth,
                  UI_UNIT_Y,
-                 cpicker->hsv,
+                 cpicker->hsv_scene_linear,
                  0.0,
                  1.0,
                  10,
@@ -706,7 +712,7 @@ static void ui_block_colorpicker(uiBlock *block, uiBut *from_but, float rgba[4],
                  yco -= UI_UNIT_Y,
                  butwidth,
                  UI_UNIT_Y,
-                 cpicker->hsv + 1,
+                 cpicker->hsv_scene_linear + 1,
                  0.0,
                  1.0,
                  10,
@@ -724,7 +730,7 @@ static void ui_block_colorpicker(uiBlock *block, uiBut *from_but, float rgba[4],
                    yco -= UI_UNIT_Y,
                    butwidth,
                    UI_UNIT_Y,
-                   cpicker->hsv + 2,
+                   cpicker->hsv_scene_linear + 2,
                    0.0,
                    1.0,
                    10,
@@ -740,7 +746,7 @@ static void ui_block_colorpicker(uiBlock *block, uiBut *from_but, float rgba[4],
                    yco -= UI_UNIT_Y,
                    butwidth,
                    UI_UNIT_Y,
-                   cpicker->hsv + 2,
+                   cpicker->hsv_scene_linear + 2,
                    0.0,
                    softmax,
                    10,
@@ -755,7 +761,7 @@ static void ui_block_colorpicker(uiBlock *block, uiBut *from_but, float rgba[4],
 
   UI_block_align_end(block);
 
-  if (rgba[3] != FLT_MAX) {
+  if (rgba_scene_linear[3] != FLT_MAX) {
     bt = uiDefButR_prop(block,
                         UI_BTYPE_NUM_SLIDER,
                         0,
@@ -776,14 +782,14 @@ static void ui_block_colorpicker(uiBlock *block, uiBut *from_but, float rgba[4],
     bt->custom_data = cpicker;
   }
   else {
-    rgba[3] = 1.0f;
+    rgba_scene_linear[3] = 1.0f;
   }
 
   /* Hex color is in sRGB space. */
   float rgb_hex[3];
   uchar rgb_hex_uchar[3];
 
-  copy_v3_v3(rgb_hex, rgba);
+  copy_v3_v3(rgb_hex, rgba_scene_linear);
 
   if (!ui_but_is_color_gamma(from_but)) {
     IMB_colormanagement_scene_linear_to_srgb_v3(rgb_hex);
@@ -846,21 +852,22 @@ static int ui_colorpicker_small_wheel_cb(const bContext *UNUSED(C),
     LISTBASE_FOREACH (uiBut *, but, &block->buttons) {
       if (but->type == UI_BTYPE_HSVCUBE && but->active == NULL) {
         uiPopupBlockHandle *popup = block->handle;
-        float rgb[3];
         ColorPicker *cpicker = but->custom_data;
-        float *hsv = cpicker->color_data;
+        float *hsv_perceptual = cpicker->hsv_perceptual;
 
-        ui_but_v3_get(but, rgb);
-        ui_scene_linear_to_color_picker_space(but, rgb);
-        ui_rgb_to_color_picker_compat_v(rgb, hsv);
+        float rgb_perceptual[3];
+        ui_but_v3_get(but, rgb_perceptual);
+        ui_scene_linear_to_perceptual_space(but, rgb_perceptual);
+        ui_color_picker_rgb_to_hsv_compat(rgb_perceptual, hsv_perceptual);
 
-        hsv[2] = clamp_f(hsv[2] + add, 0.0f, 1.0f);
+        hsv_perceptual[2] = clamp_f(hsv_perceptual[2] + add, 0.0f, 1.0f);
 
-        ui_color_picker_to_rgb_v(hsv, rgb);
-        ui_color_picker_to_scene_linear_space(but, rgb);
-        ui_but_v3_set(but, rgb);
+        float rgb_scene_linear[3];
+        ui_color_picker_hsv_to_rgb(hsv_perceptual, rgb_scene_linear);
+        ui_perceptual_to_scene_linear_space(but, rgb_scene_linear);
+        ui_but_v3_set(but, rgb_scene_linear);
 
-        ui_update_color_picker_buts_rgb(but, block, cpicker, rgb);
+        ui_update_color_picker_buts_rgb(but, block, cpicker, rgb_scene_linear);
         if (popup) {
           popup->menuretval = UI_RETURN_UPDATE;
         }