Bug 757444 - Curves and Levels should operate by default on linear RGB...

...and present linear RGB Histograms

This is step one: implement the feature at all (without new defaults
or proper GUI, cough).

Add boolean "linear" properties to GimpOperationPointFilter,
GimpCurvesConfig and GimpLevelsConfig.

In the filter, simply set the input/output formats to linear in
prepare().

In the curves and levels tools, add "Linear" toggles from hell,
like in the histogram dockable, and make sure things work right
wrt changing and resetting the property, switching from levels
to curves, and picking colors.

The result currently changes when switching a non-nop curves/levels
between perceptual and linear, because adjusting the parameters
between the spaces is not implemented yet.

app/operations/gimpcurvesconfig.c

@@ -44,6 +44,7 @@
 enum
 {
   PROP_0,
+  PROP_LINEAR,
   PROP_CHANNEL,
   PROP_CURVE
 };
@@ -99,6 +100,12 @@ gimp_curves_config_class_init (GimpCurvesConfigClass *klass)
 
   viewable_class->default_icon_name = "gimp-tool-curves";
 
+  GIMP_CONFIG_PROP_BOOLEAN (object_class, PROP_LINEAR,
+                            "linear",
+                            _("Linear"),
+                            _("Work on linear RGB"),
+                            FALSE, 0);
+
   GIMP_CONFIG_PROP_ENUM (object_class, PROP_CHANNEL,
                          "channel",
                          _("Channel"),
@@ -170,6 +177,10 @@ gimp_curves_config_get_property (GObject    *object,
 
   switch (property_id)
     {
+    case PROP_LINEAR:
+      g_value_set_boolean (value, self->linear);
+      break;
+
     case PROP_CHANNEL:
       g_value_set_enum (value, self->channel);
       break;
@@ -194,6 +205,10 @@ gimp_curves_config_set_property (GObject      *object,
 
   switch (property_id)
     {
+    case PROP_LINEAR:
+      self->linear = g_value_get_boolean (value);
+      break;
+
     case PROP_CHANNEL:
       self->channel = g_value_get_enum (value);
       g_object_notify (object, "curve");
@@ -228,7 +243,8 @@ gimp_curves_config_serialize (GimpConfig       *config,
   GimpHistogramChannel  old_channel;
   gboolean              success = TRUE;
 
-  if (! gimp_config_serialize_property_by_name (config, "time", writer))
+  if (! gimp_config_serialize_property_by_name (config, "time",   writer) ||
+      ! gimp_config_serialize_property_by_name (config, "linear", writer))
     return FALSE;
 
   old_channel = c_config->channel;
@@ -284,6 +300,9 @@ gimp_curves_config_equal (GimpConfig *a,
   GimpCurvesConfig     *config_b = GIMP_CURVES_CONFIG (b);
   GimpHistogramChannel  channel;
 
+  if (config_a->linear != config_b->linear)
+    return FALSE;
+
   for (channel = GIMP_HISTOGRAM_VALUE;
        channel <= GIMP_HISTOGRAM_ALPHA;
        channel++)
@@ -322,6 +341,7 @@ gimp_curves_config_reset (GimpConfig *config)
       gimp_curves_config_reset_channel (c_config);
     }
 
+  gimp_config_reset_property (G_OBJECT (config), "linear");
   gimp_config_reset_property (G_OBJECT (config), "channel");
 }
 
@@ -343,8 +363,10 @@ gimp_curves_config_copy (GimpConfig  *src,
                         flags);
     }
 
+  dest_config->linear  = src_config->linear;
   dest_config->channel = src_config->channel;
 
+  g_object_notify (G_OBJECT (dest), "linear");
   g_object_notify (G_OBJECT (dest), "channel");
 
   return TRUE;
@@ -594,6 +616,10 @@ gimp_curves_config_load_cruft (GimpCurvesConfig  *config,
       gimp_data_thaw (GIMP_DATA (curve));
     }
 
+  config->linear = FALSE;
+
+  g_object_notify (G_OBJECT (config), "linear");
+
   g_object_thaw_notify (G_OBJECT (config));
 
   return TRUE;

app/operations/gimpcurvesconfig.h

@@ -39,6 +39,8 @@ struct _GimpCurvesConfig
 {
   GimpSettings          parent_instance;
 
+  gboolean              linear;
+
   GimpHistogramChannel  channel;
 
   GimpCurve            *curve[5];

app/operations/gimplevelsconfig.c

@@ -46,6 +46,7 @@
 enum
 {
   PROP_0,
+  PROP_LINEAR,
   PROP_CHANNEL,
   PROP_LOW_INPUT,
   PROP_HIGH_INPUT,
@@ -102,6 +103,12 @@ gimp_levels_config_class_init (GimpLevelsConfigClass *klass)
 
   viewable_class->default_icon_name = "gimp-tool-levels";
 
+  GIMP_CONFIG_PROP_BOOLEAN (object_class, PROP_LINEAR,
+                            "linear",
+                            _("Linear"),
+                            _("Work on linear RGB"),
+                            FALSE, 0);
+
   GIMP_CONFIG_PROP_ENUM (object_class, PROP_CHANNEL,
                          "channel",
                          _("Channel"),
@@ -178,6 +185,10 @@ gimp_levels_config_get_property (GObject    *object,
 
   switch (property_id)
     {
+    case PROP_LINEAR:
+      g_value_set_boolean (value, self->linear);
+      break;
+
     case PROP_CHANNEL:
       g_value_set_enum (value, self->channel);
       break;
@@ -226,6 +237,10 @@ gimp_levels_config_set_property (GObject      *object,
 
   switch (property_id)
     {
+    case PROP_LINEAR:
+      self->linear = g_value_get_boolean (value);
+      break;
+
     case PROP_CHANNEL:
       self->channel = g_value_get_enum (value);
       g_object_notify (object, "low-input");
@@ -280,6 +295,7 @@ gimp_levels_config_serialize (GimpConfig       *config,
   gboolean              success = TRUE;
 
   if (! gimp_config_serialize_property_by_name (config, "time",         writer) ||
+      ! gimp_config_serialize_property_by_name (config, "linear",       writer) ||
       ! gimp_config_serialize_property_by_name (config, "clamp-input",  writer) ||
       ! gimp_config_serialize_property_by_name (config, "clamp-output", writer))
     return FALSE;
@@ -341,7 +357,8 @@ gimp_levels_config_equal (GimpConfig *a,
   GimpLevelsConfig     *config_b = GIMP_LEVELS_CONFIG (b);
   GimpHistogramChannel  channel;
 
-  if (config_a->clamp_input  != config_b->clamp_input ||
+  if (config_a->linear       != config_b->linear      ||
+      config_a->clamp_input  != config_b->clamp_input ||
       config_a->clamp_output != config_b->clamp_output)
     return FALSE;
 
@@ -376,6 +393,7 @@ gimp_levels_config_reset (GimpConfig *config)
       gimp_levels_config_reset_channel (l_config);
     }
 
+  gimp_config_reset_property (G_OBJECT (config), "linear");
   gimp_config_reset_property (G_OBJECT (config), "channel");
   gimp_config_reset_property (G_OBJECT (config), "clamp-input");
   gimp_config_reset_property (G_OBJECT (config), "clamp_output");
@@ -407,10 +425,12 @@ gimp_levels_config_copy (GimpConfig  *src,
   g_object_notify (G_OBJECT (dest), "low-output");
   g_object_notify (G_OBJECT (dest), "high-output");
 
+  dest_config->linear       = src_config->linear;
   dest_config->channel      = src_config->channel;
   dest_config->clamp_input  = src_config->clamp_input;
   dest_config->clamp_output = src_config->clamp_output;
 
+  g_object_notify (G_OBJECT (dest), "linear");
   g_object_notify (G_OBJECT (dest), "channel");
   g_object_notify (G_OBJECT (dest), "clamp-input");
   g_object_notify (G_OBJECT (dest), "clamp-output");
@@ -668,13 +688,15 @@ gimp_levels_config_to_curves_config (GimpLevelsConfig *config)
 
   curves = g_object_new (GIMP_TYPE_CURVES_CONFIG, NULL);
 
+  curves->linear = config->linear;
+
   for (channel = GIMP_HISTOGRAM_VALUE;
        channel <= GIMP_HISTOGRAM_ALPHA;
        channel++)
     {
       GimpCurve  *curve    = curves->curve[channel];
       const gint  n_points = gimp_curve_get_n_points (curve);
-      static const gint n  = 4;
+      static const gint n  = 8;
       gint        point    = -1;
       gdouble     gamma    = config->gamma[channel];
       gdouble     delta_in;
@@ -871,9 +893,11 @@ gimp_levels_config_load_cruft (GimpLevelsConfig  *config,
       config->high_output[i] = high_output[i] / 255.0;
     }
 
+  config->linear       = FALSE;
   config->clamp_input  = TRUE;
   config->clamp_output = TRUE;
 
+  g_object_notify (G_OBJECT (config), "linear");
   g_object_notify (G_OBJECT (config), "low-input");
   g_object_notify (G_OBJECT (config), "high-input");
   g_object_notify (G_OBJECT (config), "clamp-input");

app/operations/gimplevelsconfig.h

@@ -39,6 +39,8 @@ struct _GimpLevelsConfig
 {
   GimpSettings          parent_instance;
 
+  gboolean              linear;
+
   GimpHistogramChannel  channel;
 
   gdouble               low_input[5];

app/operations/gimpoperationcurves.c

@@ -67,6 +67,14 @@ gimp_operation_curves_class_init (GimpOperationCurvesClass *klass)
 
   point_class->process = gimp_operation_curves_process;
 
+  g_object_class_install_property (object_class,
+                                   GIMP_OPERATION_POINT_FILTER_PROP_LINEAR,
+                                   g_param_spec_boolean ("linear",
+                                                         "Linear",
+                                                         "Whether to operate on linear RGB",
+                                                         FALSE,
+                                                         G_PARAM_READWRITE));
+
   g_object_class_install_property (object_class,
                                    GIMP_OPERATION_POINT_FILTER_PROP_CONFIG,
                                    g_param_spec_object ("config",

app/operations/gimpoperationlevels.c

@@ -64,6 +64,14 @@ gimp_operation_levels_class_init (GimpOperationLevelsClass *klass)
 
   point_class->process = gimp_operation_levels_process;
 
+  g_object_class_install_property (object_class,
+                                   GIMP_OPERATION_POINT_FILTER_PROP_LINEAR,
+                                   g_param_spec_boolean ("linear",
+                                                         "Linear",
+                                                         "Whether to operate on linear RGB",
+                                                         FALSE,
+                                                         G_PARAM_READWRITE));
+
   g_object_class_install_property (object_class,
                                    GIMP_OPERATION_POINT_FILTER_PROP_CONFIG,
                                    g_param_spec_object ("config",

app/operations/gimpoperationpointfilter.c

@@ -73,6 +73,10 @@ gimp_operation_point_filter_get_property (GObject    *object,
 
   switch (property_id)
     {
+    case GIMP_OPERATION_POINT_FILTER_PROP_LINEAR:
+      g_value_set_boolean (value, self->linear);
+      break;
+
     case GIMP_OPERATION_POINT_FILTER_PROP_CONFIG:
       g_value_set_object (value, self->config);
       break;
@@ -93,6 +97,10 @@ gimp_operation_point_filter_set_property (GObject      *object,
 
   switch (property_id)
     {
+    case GIMP_OPERATION_POINT_FILTER_PROP_LINEAR:
+      self->linear = g_value_get_boolean (value);
+      break;
+
     case GIMP_OPERATION_POINT_FILTER_PROP_CONFIG:
       if (self->config)
         g_object_unref (self->config);
@@ -108,7 +116,13 @@ gimp_operation_point_filter_set_property (GObject      *object,
 static void
 gimp_operation_point_filter_prepare (GeglOperation *operation)
 {
-  const Babl *format = babl_format ("R'G'B'A float");
+  GimpOperationPointFilter *self = GIMP_OPERATION_POINT_FILTER (operation);
+  const Babl               *format;
+
+  if (self->linear)
+    format = babl_format ("RGBA float");
+  else
+    format = babl_format ("R'G'B'A float");
 
   gegl_operation_set_format (operation, "input",  format);
   gegl_operation_set_format (operation, "output", format);

app/operations/gimpoperationpointfilter.h

@@ -29,6 +29,7 @@
 enum
 {
   GIMP_OPERATION_POINT_FILTER_PROP_0,
+  GIMP_OPERATION_POINT_FILTER_PROP_LINEAR,
   GIMP_OPERATION_POINT_FILTER_PROP_CONFIG
 };
 
@@ -47,6 +48,7 @@ struct _GimpOperationPointFilter
 {
   GeglOperationPointFilter  parent_instance;
 
+  gboolean                  linear;
   GObject                  *config;
 };
 

app/tools/gimpcurvestool.c

@@ -183,18 +183,25 @@ gimp_curves_tool_initialize (GimpTool     *tool,
                              GimpDisplay  *display,
                              GError      **error)
 {
-  GimpCurvesTool *c_tool      = GIMP_CURVES_TOOL (tool);
+  GimpFilterTool   *filter_tool = GIMP_FILTER_TOOL (tool);
-  GimpFilterTool *filter_tool = GIMP_FILTER_TOOL (tool);
+  GimpCurvesTool   *c_tool      = GIMP_CURVES_TOOL (tool);
-  GimpImage      *image       = gimp_display_get_image (display);
+  GimpImage        *image       = gimp_display_get_image (display);
-  GimpDrawable   *drawable    = gimp_image_get_active_drawable (image);
+  GimpDrawable     *drawable    = gimp_image_get_active_drawable (image);
-  GimpHistogram  *histogram;
+  GimpCurvesConfig *config;
+  GimpHistogram    *histogram;
 
   if (! GIMP_TOOL_CLASS (parent_class)->initialize (tool, display, error))
     {
       return FALSE;
     }
 
-  histogram = gimp_histogram_new (FALSE);
+  config = GIMP_CURVES_CONFIG (filter_tool->config);
+
+  gegl_node_set (filter_tool->operation,
+                 "linear", config->linear,
+                 NULL);
+
+  histogram = gimp_histogram_new (config->linear);
   gimp_drawable_calculate_histogram (drawable, histogram, FALSE);
   gimp_histogram_view_set_background (GIMP_HISTOGRAM_VIEW (c_tool->graph),
                                       histogram);
@@ -436,6 +443,11 @@ gimp_curves_tool_dialog (GimpFilterTool *filter_tool)
   gtk_box_pack_end (GTK_BOX (hbox), hbox2, FALSE, FALSE, 0);
   gtk_widget_show (hbox2);
 
+  button = gimp_prop_check_button_new (G_OBJECT (config), "linear", NULL);
+  gtk_toggle_button_set_mode (GTK_TOGGLE_BUTTON (button), FALSE);
+  gtk_box_pack_end (GTK_BOX (hbox), button, FALSE, FALSE, 0);
+  gtk_widget_show (button);
+
   frame_vbox = gtk_box_new (GTK_ORIENTATION_VERTICAL, 4);
   gtk_container_add (GTK_CONTAINER (main_frame), frame_vbox);
   gtk_widget_show (frame_vbox);
@@ -545,6 +557,13 @@ gimp_curves_tool_reset (GimpFilterTool *filter_tool)
 
   default_config = GIMP_CURVES_CONFIG (filter_tool->default_config);
 
+  g_object_freeze_notify (G_OBJECT (config));
+
+  if (default_config)
+    g_object_set (config, "linear", default_config->linear, NULL);
+  else
+    gimp_config_reset_property (G_OBJECT (config), "linear");
+
   for (channel = GIMP_HISTOGRAM_VALUE;
        channel <= GIMP_HISTOGRAM_ALPHA;
        channel++)
@@ -570,6 +589,8 @@ gimp_curves_tool_reset (GimpFilterTool *filter_tool)
           gimp_curve_reset (config->curve[channel], FALSE);
         }
     }
+
+  g_object_thaw_notify (G_OBJECT (config));
 }
 
 static void
@@ -587,7 +608,22 @@ gimp_curves_tool_config_notify (GimpFilterTool   *filter_tool,
   if (! curves_tool->channel_menu)
     return;
 
-  if (! strcmp (pspec->name, "channel"))
+  if (! strcmp (pspec->name, "linear"))
+    {
+      GimpHistogram *histogram;
+
+      gegl_node_set (filter_tool->operation,
+                     "linear", curves_config->linear,
+                     NULL);
+
+      histogram = gimp_histogram_new (curves_config->linear);
+      gimp_drawable_calculate_histogram (GIMP_TOOL (filter_tool)->drawable,
+                                         histogram, FALSE);
+      gimp_histogram_view_set_background (GIMP_HISTOGRAM_VIEW (curves_tool->graph),
+                                          histogram);
+      g_object_unref (histogram);
+    }
+  else if (! strcmp (pspec->name, "channel"))
     {
       gimp_curves_tool_update_channel (GIMP_CURVES_TOOL (filter_tool));
     }
@@ -652,18 +688,23 @@ gimp_curves_tool_color_picked (GimpFilterTool *filter_tool,
   GimpCurvesTool   *tool     = GIMP_CURVES_TOOL (filter_tool);
   GimpCurvesConfig *config   = GIMP_CURVES_CONFIG (filter_tool->config);
   GimpDrawable     *drawable = GIMP_TOOL (tool)->drawable;
+  GimpRGB           rgb      = *color;
 
-  tool->picked_color[GIMP_HISTOGRAM_RED]   = color->r;
+  if (config->linear)
-  tool->picked_color[GIMP_HISTOGRAM_GREEN] = color->g;
+    babl_process (babl_fish (babl_format ("R'G'B'A double"),
-  tool->picked_color[GIMP_HISTOGRAM_BLUE]  = color->b;
+                             babl_format ("RGBA double")),
+                  &rgb, &rgb, 1);
+
+  tool->picked_color[GIMP_HISTOGRAM_RED]   = rgb.r;
+  tool->picked_color[GIMP_HISTOGRAM_GREEN] = rgb.g;
+  tool->picked_color[GIMP_HISTOGRAM_BLUE]  = rgb.b;
 
   if (gimp_drawable_has_alpha (drawable))
-    tool->picked_color[GIMP_HISTOGRAM_ALPHA] = color->a;
+    tool->picked_color[GIMP_HISTOGRAM_ALPHA] = rgb.a;
   else
     tool->picked_color[GIMP_HISTOGRAM_ALPHA] = -1;
 
-  tool->picked_color[GIMP_HISTOGRAM_VALUE] = MAX (MAX (color->r, color->g),
+  tool->picked_color[GIMP_HISTOGRAM_VALUE] = MAX (MAX (rgb.r, rgb.g), rgb.b);
-                                                  color->b);
 
   gimp_curve_view_set_xpos (GIMP_CURVE_VIEW (tool->graph),
                             tool->picked_color[config->channel]);

app/tools/gimplevelstool.c

@@ -160,7 +160,6 @@ gimp_levels_tool_class_init (GimpLevelsToolClass *klass)
 static void
 gimp_levels_tool_init (GimpLevelsTool *tool)
 {
-  tool->histogram = gimp_histogram_new (FALSE);
 }
 
 static void
@@ -178,9 +177,11 @@ gimp_levels_tool_initialize (GimpTool     *tool,
                              GimpDisplay  *display,
                              GError      **error)
 {
-  GimpLevelsTool   *l_tool   = GIMP_LEVELS_TOOL (tool);
+  GimpFilterTool   *filter_tool = GIMP_FILTER_TOOL (tool);
-  GimpImage        *image    = gimp_display_get_image (display);
+  GimpLevelsTool   *l_tool      = GIMP_LEVELS_TOOL (tool);
-  GimpDrawable     *drawable = gimp_image_get_active_drawable (image);
+  GimpImage        *image       = gimp_display_get_image (display);
+  GimpDrawable     *drawable    = gimp_image_get_active_drawable (image);
+  GimpLevelsConfig *config;
   gdouble           scale_factor;
   gdouble           step_increment;
   gdouble           page_increment;
@@ -191,6 +192,15 @@ gimp_levels_tool_initialize (GimpTool     *tool,
       return FALSE;
     }
 
+  config = GIMP_LEVELS_CONFIG (filter_tool->config);
+
+  gegl_node_set (filter_tool->operation,
+                 "linear", config->linear,
+                 NULL);
+
+  g_clear_object (&l_tool->histogram);
+  l_tool->histogram = gimp_histogram_new (config->linear);
+
   gimp_drawable_calculate_histogram (drawable, l_tool->histogram, FALSE);
   gimp_histogram_view_set_histogram (GIMP_HISTOGRAM_VIEW (l_tool->histogram_view),
                                      l_tool->histogram);
@@ -376,6 +386,11 @@ gimp_levels_tool_dialog (GimpFilterTool *filter_tool)
   gtk_box_pack_end (GTK_BOX (hbox), hbox2, FALSE, FALSE, 0);
   gtk_widget_show (hbox2);
 
+  button = gimp_prop_check_button_new (G_OBJECT (config), "linear", NULL);
+  gtk_toggle_button_set_mode (GTK_TOGGLE_BUTTON (button), FALSE);
+  gtk_box_pack_end (GTK_BOX (hbox), button, FALSE, FALSE, 0);
+  gtk_widget_show (button);
+
   frame_vbox = gtk_box_new (GTK_ORIENTATION_VERTICAL, 4);
   gtk_container_add (GTK_CONTAINER (main_frame), frame_vbox);
   gtk_widget_show (frame_vbox);
@@ -654,7 +669,21 @@ gimp_levels_tool_config_notify (GimpFilterTool   *filter_tool,
   if (! levels_tool->channel_menu)
     return;
 
-  if (! strcmp (pspec->name, "channel"))
+  if (! strcmp (pspec->name, "linear"))
+    {
+      gegl_node_set (filter_tool->operation,
+                     "linear", levels_config->linear,
+                     NULL);
+
+      g_clear_object (&levels_tool->histogram);
+      levels_tool->histogram = gimp_histogram_new (levels_config->linear);
+
+      gimp_drawable_calculate_histogram (GIMP_TOOL (filter_tool)->drawable,
+                                         levels_tool->histogram, FALSE);
+      gimp_histogram_view_set_histogram (GIMP_HISTOGRAM_VIEW (levels_tool->histogram_view),
+                                         levels_tool->histogram);
+    }
+  else if (! strcmp (pspec->name, "channel"))
     {
       gimp_histogram_view_set_channel (GIMP_HISTOGRAM_VIEW (levels_tool->histogram_view),
                                        levels_config->channel);
@@ -763,8 +792,14 @@ gimp_levels_tool_color_picked (GimpFilterTool *color_tool,
 {
   GimpFilterTool   *filter_tool = GIMP_FILTER_TOOL (color_tool);
   GimpLevelsConfig *config      = GIMP_LEVELS_CONFIG (filter_tool->config);
+  GimpRGB           rgb         = *color;
   guint             value       = GPOINTER_TO_UINT (identifier);
 
+  if (config->linear)
+    babl_process (babl_fish (babl_format ("R'G'B'A double"),
+                             babl_format ("RGBA double")),
+                  &rgb, &rgb, 1);
+
   if (value & PICK_ALL_CHANNELS &&
       gimp_babl_format_get_base_type (sample_format) == GIMP_RGB)
     {
@@ -791,12 +826,12 @@ gimp_levels_tool_color_picked (GimpFilterTool *color_tool,
            channel <= GIMP_HISTOGRAM_BLUE;
            channel++)
         {
-          levels_input_adjust_by_color (config, value, channel, color);
+          levels_input_adjust_by_color (config, value, channel, &rgb);
         }
     }
   else
     {
-      levels_input_adjust_by_color (config, value, config->channel, color);
+      levels_input_adjust_by_color (config, value, config->channel, &rgb);
     }
 }