Fix T39228 Gamma/lift/gain are burned out in the circular color pickers
authorAntony Riakiotakis <kalast@gmail.com>
Mon, 17 Mar 2014 23:08:24 +0000 (01:08 +0200)
committerAntony Riakiotakis <kalast@gmail.com>
Mon, 17 Mar 2014 23:08:36 +0000 (01:08 +0200)
and value/lightness slider stops midway.

Issue here is manyfold:

Color wheel does not support properties with different soft min/max
values than 1.0 (which after experimenting a little I left as is), and
also color management is completely destroying the mapping between the
value slider and the RNA property value range. To solve this I have
disabled color management by setting the property to gamma corrected
(only in RNA, Sequence editor coders please check!), otherwise it will
just become a big mess of tracking where color comes from and what kind
of color transforms it needs in different color pickers (if property has
non normalized range etc). HSL is not really meant to represent colors
outside a normalized space so I have disabled setting lightness above
1.0 in this model.

This will work, however it is hacking a color picker to do something
other than what it is supposed to do: pick a color from the screen
accurately. Which means normalized values always. The non normalized
colors picked for lift/gain/gamma through the pickers do not correspond
to any accurate colors; they are rather a user friendly way to 'sort of'
choose a color and a gamma with an indication of maximum value.

I think that lift/gamma/gain nodes need a dedicated widget for this
(besides it is quite clear that some options are written for that use
case) -or- a separate gamma multiplier for the picked color (which
should itself be in a normalized space)

source/blender/blenlib/intern/math_color.c
source/blender/editors/interface/interface_handlers.c
source/blender/editors/interface/interface_regions.c
source/blender/editors/interface/interface_widgets.c
source/blender/makesrna/intern/rna_sequencer.c

index 093b82e9126a1dd21b26b28f135a30d0fac9ae6f..61d55c720b0d676d6d9ee582080b8d9c0d3190eb 100644 (file)
@@ -349,7 +349,7 @@ void rgb_to_hsl(float r, float g, float b, float *lh, float *ls, float *ll)
 {
        const float cmax = max_fff(r, g, b);
        const float cmin = min_fff(r, g, b);
-       float h, s, l = (cmax + cmin) / 2.0f;
+       float h, s, l = min_ff(1.0, (cmax + cmin) / 2.0f);
 
        if (cmax == cmin) {
                h = s = 0.0f; // achromatic
index 11e361d0311a314e6cde534918da36452aba0645..03c19f5e94bed28408bd22766463f1282c05e686 100644 (file)
@@ -4369,8 +4369,10 @@ static bool ui_numedit_but_HSVCUBE(uiBut *but, uiHandleButtonData *data,
                case UI_GRAD_V:
                        hsv[2] = x;
                        break;
-               case UI_GRAD_V_ALT:
                case UI_GRAD_L_ALT:
+                       hsv[2] = y;
+                       break;
+               case UI_GRAD_V_ALT:
                        /* vertical 'value' strip */
 
                        /* exception only for value strip - use the range set in but->min/max */
@@ -4393,7 +4395,7 @@ static bool ui_numedit_but_HSVCUBE(uiBut *but, uiHandleButtonData *data,
                ui_block_to_scene_linear_v3(but->block, rgb);
 
        /* clamp because with color conversion we can exceed range [#34295] */
-       if (ELEM((int)but->a1, UI_GRAD_V_ALT, UI_GRAD_L_ALT)) {
+       if (but->a1 == UI_GRAD_V_ALT) {
                clamp_axis_max_v3(rgb, but->softmax);
        }
 
@@ -4662,7 +4664,7 @@ static bool ui_numedit_but_HSVCIRCLE(uiBut *but, uiHandleButtonData *data,
        
        ui_hsvcircle_vals_from_pos(hsv, hsv + 1, &rect, mx_fl, my_fl);
 
-       if (but->flag & UI_BUT_COLOR_CUBIC)
+       if ((but->flag & UI_BUT_COLOR_CUBIC) && (U.color_picker_type == USER_CP_CIRCLE_HSV))
                hsv[1] = 1.0f - sqrt3f(1.0f - hsv[1]);
 
        if (snap != SNAP_OFF) {
index cd483adac5337d455c2dfcb464d69968ff4179c6..c6d9769d918fa8b2c0570d8410247a722fe867b4 100644 (file)
@@ -1955,7 +1955,7 @@ static void uiBlockPicker(uiBlock *block, float rgba[4], PointerRNA *ptr, Proper
        bt = uiDefButF(block, NUMSLI, 0, IFACE_("S:"),   0, yco -= UI_UNIT_Y, butwidth, UI_UNIT_Y, hsv + 1, 0.0, 1.0, 10, 3, TIP_("Saturation"));
        uiButSetFunc(bt, do_color_wheel_rna_cb, bt, hsv);
        if (U.color_picker_type == USER_CP_CIRCLE_HSL)
-               bt = uiDefButF(block, NUMSLI, 0, IFACE_("L:"),   0, yco -= UI_UNIT_Y, butwidth, UI_UNIT_Y, hsv + 2, 0.0, softmax, 10, 3, TIP_("Lightness"));
+               bt = uiDefButF(block, NUMSLI, 0, IFACE_("L:"),   0, yco -= UI_UNIT_Y, butwidth, UI_UNIT_Y, hsv + 2, 0.0, 1.0, 10, 3, TIP_("Lightness"));
        else
                bt = uiDefButF(block, NUMSLI, 0, IFACE_("V:"),   0, yco -= UI_UNIT_Y, butwidth, UI_UNIT_Y, hsv + 2, 0.0, softmax, 10, 3, TIP_("Value"));
 
index 179ec578b1d06946ccaaf4d9662f57d8a6249ca4..2eff3a359babeb0ce40b3651972a8a4bb975f654 100644 (file)
@@ -2027,7 +2027,7 @@ void ui_hsvcircle_pos_from_vals(uiBut *but, const rcti *rect, float *hsv, float
        
        ang = 2.0f * (float)M_PI * hsv[0] + 0.5f * (float)M_PI;
        
-       if (but->flag & UI_BUT_COLOR_CUBIC)
+       if ((but->flag & UI_BUT_COLOR_CUBIC) && (U.color_picker_type == USER_CP_CIRCLE_HSV))
                radius_t = (1.0f - powf(1.0f - hsv[1], 3.0f));
        else
                radius_t = hsv[1];
@@ -2063,10 +2063,11 @@ static void ui_draw_but_HSVCIRCLE(uiBut *but, uiWidgetColors *wcol, const rcti *
        if (color_profile)
                ui_block_to_display_space_v3(but->block, rgb);
 
-       ui_rgb_to_color_picker_compat_v(rgb, hsvo);
-
        ui_rgb_to_color_picker_compat_v(rgb, hsv);
-       
+       copy_v3_v3(hsvo, hsv);
+
+       CLAMP(hsv[2], 0.0f, 1.0f); /* for display only */
+
        /* exception: if 'lock' is set
         * lock the value of the color wheel to 1.
         * Useful for color correction tools where you're only interested in hue. */
@@ -2086,7 +2087,6 @@ static void ui_draw_but_HSVCIRCLE(uiBut *but, uiWidgetColors *wcol, const rcti *
                float co = cos(ang);
                
                ui_hsvcircle_vals_from_pos(hsv, hsv + 1, rect, centx + co * radius, centy + si * radius);
-               CLAMP(hsv[2], 0.0f, 1.0f); /* for display only */
 
                ui_color_picker_to_rgb_v(hsv, col);
 
@@ -2287,8 +2287,12 @@ void ui_hsvcube_pos_from_vals(uiBut *but, const rcti *rect, float *hsv, float *x
                        x = hsv[1]; y = 0.5; break;
                case UI_GRAD_V:
                        x = hsv[2]; y = 0.5; break;
-               case UI_GRAD_V_ALT:
                case UI_GRAD_L_ALT:
+                       x = 0.5f;
+                       /* exception only for value strip - use the range set in but->min/max */
+                       y = hsv[2];
+                       break;
+               case UI_GRAD_V_ALT:
                        x = 0.5f;
                        /* exception only for value strip - use the range set in but->min/max */
                        y = (hsv[2] - but->softmin ) / (but->softmax - but->softmin);
@@ -2337,7 +2341,7 @@ static void ui_draw_but_HSV_v(uiBut *but, const rcti *rect)
        uiWidgetBase wtb;
        const float rad = 0.5f * BLI_rcti_size_x(rect);
        float x, y;
-       float rgb[3], hsv[3], v, range;
+       float rgb[3], hsv[3], v;
        bool color_profile = but->block->color_profile;
        
        if (but->rnaprop && RNA_property_subtype(but->rnaprop) == PROP_COLOR_GAMMA)
@@ -2355,9 +2359,11 @@ static void ui_draw_but_HSV_v(uiBut *but, const rcti *rect)
        v = hsv[2];
        
        /* map v from property range to [0,1] */
-       range = but->softmax - but->softmin;
-       v = (v - but->softmin) / range;
-       
+       if (but->a1 == UI_GRAD_V_ALT) {
+               float range = but->softmax - but->softmin;
+               v = (v - but->softmin) / range;
+       }
+
        widget_init(&wtb);
        
        /* fully rounded */
index 97f7ee3bbbb746962bb7ec8d66c6c97224538cae..14149da78e8ea6231dcc39f5ab3db8b0c48bc156 100644 (file)
@@ -1227,19 +1227,19 @@ static void rna_def_color_balance(BlenderRNA *brna)
        RNA_def_struct_ui_text(srna, "Sequence Color Balance Data", "Color balance parameters for a sequence strip and it's modifiers");
        RNA_def_struct_sdna(srna, "StripColorBalance");
 
-       prop = RNA_def_property(srna, "lift", PROP_FLOAT, PROP_COLOR);
+       prop = RNA_def_property(srna, "lift", PROP_FLOAT, PROP_COLOR_GAMMA);
        RNA_def_property_ui_text(prop, "Lift", "Color balance lift (shadows)");
        RNA_def_property_ui_range(prop, 0, 2, 0.1, 3);
        RNA_def_property_float_default(prop, 1.0f);
        RNA_def_property_update(prop, NC_SCENE | ND_SEQUENCER, "rna_SequenceColorBalance_update");
 
-       prop = RNA_def_property(srna, "gamma", PROP_FLOAT, PROP_COLOR);
+       prop = RNA_def_property(srna, "gamma", PROP_FLOAT, PROP_COLOR_GAMMA);
        RNA_def_property_ui_text(prop, "Gamma", "Color balance gamma (midtones)");
        RNA_def_property_ui_range(prop, 0, 2, 0.1, 3);
        RNA_def_property_float_default(prop, 1.0f);
        RNA_def_property_update(prop, NC_SCENE | ND_SEQUENCER, "rna_SequenceColorBalance_update");
 
-       prop = RNA_def_property(srna, "gain", PROP_FLOAT, PROP_COLOR);
+       prop = RNA_def_property(srna, "gain", PROP_FLOAT, PROP_COLOR_GAMMA);
        RNA_def_property_ui_text(prop, "Gain", "Color balance gain (highlights)");
        RNA_def_property_ui_range(prop, 0, 2, 0.1, 3);
        RNA_def_property_float_default(prop, 1.0f);