From dce93c7d7e012ba9b85766e13d4f6dd62e21ec8d Mon Sep 17 00:00:00 2001 From: Michael Natterer Date: Tue, 2 Jan 2018 18:42:21 +0100 Subject: [PATCH] Bug 762443 - Levels tool Output Level sliders works incorrectly Add "clamp-input" (which clamps the input values to [0..1]) and "clamp-output" (which clips the final result to [0..1]), properties, parameters and GUI to: - GimpLevelsConfig - GimpOperationLevels - The levels tool dialog - The gimp_drawable_levels() PDB API The old deprecated gimp_levels() PDB API now sets both clamping options to TRUE which restores the 2.8 behavior. Also reorder some stuff in GimpLevelsConfig and elsewhere so the levels parameters are always in the same order. --- app/operations/gimplevelsconfig.c | 91 +++++++++++++++++++++------- app/operations/gimplevelsconfig.h | 8 ++- app/operations/gimpoperationlevels.c | 32 +++++++--- app/pdb/color-cmds.c | 12 ++-- app/pdb/drawable-color-cmds.c | 34 ++++++++--- app/tools/gimplevelstool.c | 19 +++++- libgimp/gimpdrawablecolor_pdb.c | 8 ++- libgimp/gimpdrawablecolor_pdb.h | 4 +- pdb/groups/color.pdb | 12 ++-- pdb/groups/drawable_color.pdb | 18 ++++-- 10 files changed, 176 insertions(+), 62 deletions(-) diff --git a/app/operations/gimplevelsconfig.c b/app/operations/gimplevelsconfig.c index b6c9339ffc..00a471f634 100644 --- a/app/operations/gimplevelsconfig.c +++ b/app/operations/gimplevelsconfig.c @@ -47,11 +47,13 @@ enum { PROP_0, PROP_CHANNEL, - PROP_GAMMA, PROP_LOW_INPUT, PROP_HIGH_INPUT, + PROP_CLAMP_INPUT, + PROP_GAMMA, PROP_LOW_OUTPUT, - PROP_HIGH_OUTPUT + PROP_HIGH_OUTPUT, + PROP_CLAMP_OUTPUT }; @@ -107,12 +109,6 @@ gimp_levels_config_class_init (GimpLevelsConfigClass *klass) GIMP_TYPE_HISTOGRAM_CHANNEL, GIMP_HISTOGRAM_VALUE, 0); - GIMP_CONFIG_PROP_DOUBLE (object_class, PROP_GAMMA, - "gamma", - _("Gamma"), - _("Gamma"), - 0.1, 10.0, 1.0, 0); - GIMP_CONFIG_PROP_DOUBLE (object_class, PROP_LOW_INPUT, "low-input", _("Low Input"), @@ -125,6 +121,18 @@ gimp_levels_config_class_init (GimpLevelsConfigClass *klass) _("High Input"), 0.0, 1.0, 1.0, 0); + GIMP_CONFIG_PROP_BOOLEAN (object_class, PROP_CLAMP_INPUT, + "clamp-input", + _("Clamp Input"), + _("Clamp input values before applying output mapping."), + FALSE, 0); + + GIMP_CONFIG_PROP_DOUBLE (object_class, PROP_GAMMA, + "gamma", + _("Gamma"), + _("Gamma"), + 0.1, 10.0, 1.0, 0); + GIMP_CONFIG_PROP_DOUBLE (object_class, PROP_LOW_OUTPUT, "low-output", _("Low Output"), @@ -136,6 +144,12 @@ gimp_levels_config_class_init (GimpLevelsConfigClass *klass) _("High Output"), _("High Output"), 0.0, 1.0, 1.0, 0); + + GIMP_CONFIG_PROP_BOOLEAN (object_class, PROP_CLAMP_OUTPUT, + "clamp-output", + _("Clamp Output"), + _("Clamp final output values."), + FALSE, 0); } static void @@ -168,10 +182,6 @@ gimp_levels_config_get_property (GObject *object, g_value_set_enum (value, self->channel); break; - case PROP_GAMMA: - g_value_set_double (value, self->gamma[self->channel]); - break; - case PROP_LOW_INPUT: g_value_set_double (value, self->low_input[self->channel]); break; @@ -180,6 +190,14 @@ gimp_levels_config_get_property (GObject *object, g_value_set_double (value, self->high_input[self->channel]); break; + case PROP_CLAMP_INPUT: + g_value_set_boolean (value, self->clamp_input); + break; + + case PROP_GAMMA: + g_value_set_double (value, self->gamma[self->channel]); + break; + case PROP_LOW_OUTPUT: g_value_set_double (value, self->low_output[self->channel]); break; @@ -188,6 +206,10 @@ gimp_levels_config_get_property (GObject *object, g_value_set_double (value, self->high_output[self->channel]); break; + case PROP_CLAMP_OUTPUT: + g_value_set_boolean (value, self->clamp_output); + break; + default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec); break; @@ -206,17 +228,13 @@ gimp_levels_config_set_property (GObject *object, { case PROP_CHANNEL: self->channel = g_value_get_enum (value); - g_object_notify (object, "gamma"); g_object_notify (object, "low-input"); g_object_notify (object, "high-input"); + g_object_notify (object, "gamma"); g_object_notify (object, "low-output"); g_object_notify (object, "high-output"); break; - case PROP_GAMMA: - self->gamma[self->channel] = g_value_get_double (value); - break; - case PROP_LOW_INPUT: self->low_input[self->channel] = g_value_get_double (value); break; @@ -225,6 +243,14 @@ gimp_levels_config_set_property (GObject *object, self->high_input[self->channel] = g_value_get_double (value); break; + case PROP_CLAMP_INPUT: + self->clamp_input = g_value_get_boolean (value); + break; + + case PROP_GAMMA: + self->gamma[self->channel] = g_value_get_double (value); + break; + case PROP_LOW_OUTPUT: self->low_output[self->channel] = g_value_get_double (value); break; @@ -233,6 +259,10 @@ gimp_levels_config_set_property (GObject *object, self->high_output[self->channel] = g_value_get_double (value); break; + case PROP_CLAMP_OUTPUT: + self->clamp_output = g_value_get_boolean (value); + break; + default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec); break; @@ -249,7 +279,9 @@ gimp_levels_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, "clamp-input", writer) || + ! gimp_config_serialize_property_by_name (config, "clamp-output", writer)) return FALSE; old_channel = l_config->channel; @@ -267,9 +299,9 @@ gimp_levels_config_serialize (GimpConfig *config, */ success = (gimp_config_serialize_property_by_name (config, "channel", writer) && - gimp_config_serialize_property_by_name (config, "gamma", writer) && gimp_config_serialize_property_by_name (config, "low-input", writer) && gimp_config_serialize_property_by_name (config, "high-input", writer) && + gimp_config_serialize_property_by_name (config, "gamma", writer) && gimp_config_serialize_property_by_name (config, "low-output", writer) && gimp_config_serialize_property_by_name (config, "high-output", writer)); @@ -309,6 +341,10 @@ gimp_levels_config_equal (GimpConfig *a, GimpLevelsConfig *config_b = GIMP_LEVELS_CONFIG (b); GimpHistogramChannel channel; + if (config_a->clamp_input != config_b->clamp_input || + config_a->clamp_output != config_b->clamp_output) + return FALSE; + for (channel = GIMP_HISTOGRAM_VALUE; channel <= GIMP_HISTOGRAM_ALPHA; channel++) @@ -341,6 +377,8 @@ gimp_levels_config_reset (GimpConfig *config) } 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"); } static gboolean @@ -369,9 +407,13 @@ gimp_levels_config_copy (GimpConfig *src, g_object_notify (G_OBJECT (dest), "low-output"); g_object_notify (G_OBJECT (dest), "high-output"); - dest_config->channel = src_config->channel; + 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), "channel"); + g_object_notify (G_OBJECT (dest), "clamp-input"); + g_object_notify (G_OBJECT (dest), "clamp-output"); return TRUE; } @@ -824,16 +866,21 @@ gimp_levels_config_load_cruft (GimpLevelsConfig *config, { config->low_input[i] = low_input[i] / 255.0; config->high_input[i] = high_input[i] / 255.0; + config->gamma[i] = gamma[i]; config->low_output[i] = low_output[i] / 255.0; config->high_output[i] = high_output[i] / 255.0; - config->gamma[i] = gamma[i]; } - g_object_notify (G_OBJECT (config), "gamma"); + config->clamp_input = TRUE; + config->clamp_output = TRUE; + g_object_notify (G_OBJECT (config), "low-input"); g_object_notify (G_OBJECT (config), "high-input"); + g_object_notify (G_OBJECT (config), "clamp-input"); + g_object_notify (G_OBJECT (config), "gamma"); g_object_notify (G_OBJECT (config), "low-output"); g_object_notify (G_OBJECT (config), "high-output"); + g_object_notify (G_OBJECT (config), "clamp-output"); g_object_thaw_notify (G_OBJECT (config)); diff --git a/app/operations/gimplevelsconfig.h b/app/operations/gimplevelsconfig.h index bac413732a..9d696cbabb 100644 --- a/app/operations/gimplevelsconfig.h +++ b/app/operations/gimplevelsconfig.h @@ -41,13 +41,17 @@ struct _GimpLevelsConfig GimpHistogramChannel channel; - gdouble gamma[5]; - gdouble low_input[5]; gdouble high_input[5]; + gboolean clamp_input; + + gdouble gamma[5]; + gdouble low_output[5]; gdouble high_output[5]; + + gboolean clamp_output; }; struct _GimpLevelsConfigClass diff --git a/app/operations/gimpoperationlevels.c b/app/operations/gimpoperationlevels.c index 415a9da88e..7385c97c42 100644 --- a/app/operations/gimpoperationlevels.c +++ b/app/operations/gimpoperationlevels.c @@ -80,12 +80,14 @@ gimp_operation_levels_init (GimpOperationLevels *self) } static inline gdouble -gimp_operation_levels_map (gdouble value, - gdouble inv_gamma, - gdouble low_input, - gdouble high_input, - gdouble low_output, - gdouble high_output) +gimp_operation_levels_map (gdouble value, + gdouble low_input, + gdouble high_input, + gboolean clamp_input, + gdouble inv_gamma, + gdouble low_output, + gdouble high_output, + gboolean clamp_output) { /* determine input intensity */ if (high_input != low_input) @@ -93,6 +95,9 @@ gimp_operation_levels_map (gdouble value, else value = (value - low_input); + if (clamp_input) + value = CLAMP (value, 0.0, 1.0); + if (inv_gamma != 1.0 && value > 0) value = pow (value, inv_gamma); @@ -102,6 +107,9 @@ gimp_operation_levels_map (gdouble value, else if (high_output < low_output) value = low_output - value * (low_output - high_output); + if (clamp_output) + value = CLAMP (value, 0.0, 1.0); + return value; } @@ -137,20 +145,24 @@ gimp_operation_levels_process (GeglOperation *operation, gdouble value; value = gimp_operation_levels_map (src[channel], - inv_gamma[channel + 1], config->low_input[channel + 1], config->high_input[channel + 1], + config->clamp_input, + inv_gamma[channel + 1], config->low_output[channel + 1], - config->high_output[channel + 1]); + config->high_output[channel + 1], + config->clamp_output); /* don't apply the overall curve to the alpha channel */ if (channel != ALPHA) value = gimp_operation_levels_map (value, - inv_gamma[0], config->low_input[0], config->high_input[0], + config->clamp_input, + inv_gamma[0], config->low_output[0], - config->high_output[0]); + config->high_output[0], + config->clamp_output); dest[channel] = value; } diff --git a/app/pdb/color-cmds.c b/app/pdb/color-cmds.c index d515ce34cf..5019c7c992 100644 --- a/app/pdb/color-cmds.c +++ b/app/pdb/color-cmds.c @@ -135,11 +135,13 @@ levels_invoker (GimpProcedure *procedure, NULL); g_object_set (config, - "low-input", low_input / 255.0, - "high-input", high_input / 255.0, - "gamma", gamma, - "low-output", low_output / 255.0, - "high-output", high_output / 255.0, + "low-input", low_input / 255.0, + "high-input", high_input / 255.0, + "clamp-input", TRUE, + "gamma", gamma, + "low-output", low_output / 255.0, + "high-output", high_output / 255.0, + "clamp-input", TRUE, NULL); gimp_drawable_apply_operation_by_name (drawable, progress, diff --git a/app/pdb/drawable-color-cmds.c b/app/pdb/drawable-color-cmds.c index 22836ef148..dd3affdfec 100644 --- a/app/pdb/drawable-color-cmds.c +++ b/app/pdb/drawable-color-cmds.c @@ -566,17 +566,21 @@ drawable_levels_invoker (GimpProcedure *procedure, gint32 channel; gdouble low_input; gdouble high_input; + gboolean clamp_input; gdouble gamma; gdouble low_output; gdouble high_output; + gboolean clamp_output; drawable = gimp_value_get_drawable (gimp_value_array_index (args, 0), gimp); channel = g_value_get_enum (gimp_value_array_index (args, 1)); low_input = g_value_get_double (gimp_value_array_index (args, 2)); high_input = g_value_get_double (gimp_value_array_index (args, 3)); - gamma = g_value_get_double (gimp_value_array_index (args, 4)); - low_output = g_value_get_double (gimp_value_array_index (args, 5)); - high_output = g_value_get_double (gimp_value_array_index (args, 6)); + clamp_input = g_value_get_boolean (gimp_value_array_index (args, 4)); + gamma = g_value_get_double (gimp_value_array_index (args, 5)); + low_output = g_value_get_double (gimp_value_array_index (args, 6)); + high_output = g_value_get_double (gimp_value_array_index (args, 7)); + clamp_output = g_value_get_boolean (gimp_value_array_index (args, 8)); if (success) { @@ -593,11 +597,13 @@ drawable_levels_invoker (GimpProcedure *procedure, NULL); g_object_set (config, - "low-input", low_input, - "high-input", high_input, - "gamma", gamma, - "low-output", low_output, - "high-output", high_output, + "low-input", low_input, + "high-input", high_input, + "clamp-input", clamp_input, + "gamma", gamma, + "low-output", low_output, + "high-output", high_output, + "clamp-output", clamp_output, NULL); gimp_drawable_apply_operation_by_name (drawable, progress, @@ -1205,6 +1211,12 @@ register_drawable_color_procs (GimpPDB *pdb) "Intensity of highest input", 0.0, 1.0, 0.0, GIMP_PARAM_READWRITE)); + gimp_procedure_add_argument (procedure, + g_param_spec_boolean ("clamp-input", + "clamp input", + "Clamp input values before applying output levels", + FALSE, + GIMP_PARAM_READWRITE)); gimp_procedure_add_argument (procedure, g_param_spec_double ("gamma", "gamma", @@ -1223,6 +1235,12 @@ register_drawable_color_procs (GimpPDB *pdb) "Intensity of highest output", 0.0, 1.0, 0.0, GIMP_PARAM_READWRITE)); + gimp_procedure_add_argument (procedure, + g_param_spec_boolean ("clamp-output", + "clamp output", + "Clamp final output values", + FALSE, + GIMP_PARAM_READWRITE)); gimp_pdb_register_procedure (pdb, procedure); g_object_unref (procedure); diff --git a/app/tools/gimplevelstool.c b/app/tools/gimplevelstool.c index e3bcb4aa98..643456c9db 100644 --- a/app/tools/gimplevelstool.c +++ b/app/tools/gimplevelstool.c @@ -458,10 +458,20 @@ gimp_levels_tool_dialog (GimpFilterTool *filter_tool) gimp_handle_bar_set_adjustment (GIMP_HANDLE_BAR (handle_bar), 0, tool->low_input); + /* clamp input toggle */ + hbox2 = gtk_box_new (GTK_ORIENTATION_HORIZONTAL, 2); + gtk_box_pack_start (GTK_BOX (hbox), hbox2, TRUE, FALSE, 0); + gtk_widget_show (hbox2); + + button = gimp_prop_check_button_new (filter_tool->config, "clamp-input", + _("Clamp _input")); + gtk_box_pack_start (GTK_BOX (hbox2), button, FALSE, FALSE, 0); + gtk_widget_show (button); + /* input gamma spin */ spinbutton = gimp_prop_spin_button_new (filter_tool->config, "gamma", 0.01, 0.1, 2); - gtk_box_pack_start (GTK_BOX (hbox), spinbutton, TRUE, FALSE, 0); + gtk_box_pack_start (GTK_BOX (hbox2), spinbutton, FALSE, FALSE, 0); gimp_help_set_help_data (spinbutton, _("Gamma"), NULL); gtk_widget_show (spinbutton); @@ -543,6 +553,12 @@ gimp_levels_tool_dialog (GimpFilterTool *filter_tool) adjustment = gtk_spin_button_get_adjustment (GTK_SPIN_BUTTON (spinbutton)); gimp_handle_bar_set_adjustment (GIMP_HANDLE_BAR (handle_bar), 0, adjustment); + /* clamp output toggle */ + button = gimp_prop_check_button_new (filter_tool->config, "clamp-output", + _("Clamp outpu_t")); + gtk_box_pack_start (GTK_BOX (hbox), button, TRUE, FALSE, 0); + gtk_widget_show (button); + /* high output spin */ tool->high_output_spinbutton = spinbutton = gimp_prop_spin_button_new (filter_tool->config, "high-output", @@ -553,7 +569,6 @@ gimp_levels_tool_dialog (GimpFilterTool *filter_tool) adjustment = gtk_spin_button_get_adjustment (GTK_SPIN_BUTTON (spinbutton)); gimp_handle_bar_set_adjustment (GIMP_HANDLE_BAR (handle_bar), 2, adjustment); - /* all channels frame */ main_frame = gimp_frame_new (_("All Channels")); gtk_box_pack_start (GTK_BOX (main_vbox), main_frame, FALSE, FALSE, 0); diff --git a/libgimp/gimpdrawablecolor_pdb.c b/libgimp/gimpdrawablecolor_pdb.c index dc57970ef6..e637ce0d4b 100644 --- a/libgimp/gimpdrawablecolor_pdb.c +++ b/libgimp/gimpdrawablecolor_pdb.c @@ -514,9 +514,11 @@ gimp_drawable_invert (gint32 drawable_ID, * @channel: The channel to modify. * @low_input: Intensity of lowest input. * @high_input: Intensity of highest input. + * @clamp_input: Clamp input values before applying output levels. * @gamma: Gamma adjustment factor. * @low_output: Intensity of lowest output. * @high_output: Intensity of highest output. + * @clamp_output: Clamp final output values. * * Modifies intensity levels in the specified drawable. * @@ -542,9 +544,11 @@ gimp_drawable_levels (gint32 drawable_ID, GimpHistogramChannel channel, gdouble low_input, gdouble high_input, + gboolean clamp_input, gdouble gamma, gdouble low_output, - gdouble high_output) + gdouble high_output, + gboolean clamp_output) { GimpParam *return_vals; gint nreturn_vals; @@ -556,9 +560,11 @@ gimp_drawable_levels (gint32 drawable_ID, GIMP_PDB_INT32, channel, GIMP_PDB_FLOAT, low_input, GIMP_PDB_FLOAT, high_input, + GIMP_PDB_INT32, clamp_input, GIMP_PDB_FLOAT, gamma, GIMP_PDB_FLOAT, low_output, GIMP_PDB_FLOAT, high_output, + GIMP_PDB_INT32, clamp_output, GIMP_PDB_END); success = return_vals[0].data.d_status == GIMP_PDB_SUCCESS; diff --git a/libgimp/gimpdrawablecolor_pdb.h b/libgimp/gimpdrawablecolor_pdb.h index 6e289f9873..ae10d9692b 100644 --- a/libgimp/gimpdrawablecolor_pdb.h +++ b/libgimp/gimpdrawablecolor_pdb.h @@ -79,9 +79,11 @@ gboolean gimp_drawable_levels (gint32 drawable_ID, GimpHistogramChannel channel, gdouble low_input, gdouble high_input, + gboolean clamp_input, gdouble gamma, gdouble low_output, - gdouble high_output); + gdouble high_output, + gboolean clamp_output); gboolean gimp_drawable_levels_stretch (gint32 drawable_ID); gboolean gimp_drawable_posterize (gint32 drawable_ID, gint levels); diff --git a/pdb/groups/color.pdb b/pdb/groups/color.pdb index 0091e3688f..176e4b2d74 100644 --- a/pdb/groups/color.pdb +++ b/pdb/groups/color.pdb @@ -91,11 +91,13 @@ sub levels { NULL); g_object_set (config, - "low-input", low_input / 255.0, - "high-input", high_input / 255.0, - "gamma", gamma, - "low-output", low_output / 255.0, - "high-output", high_output / 255.0, + "low-input", low_input / 255.0, + "high-input", high_input / 255.0, + "clamp-input", TRUE, + "gamma", gamma, + "low-output", low_output / 255.0, + "high-output", high_output / 255.0, + "clamp-input", TRUE, NULL); gimp_drawable_apply_operation_by_name (drawable, progress, diff --git a/pdb/groups/drawable_color.pdb b/pdb/groups/drawable_color.pdb index 3d8a74564f..c548c8cf71 100644 --- a/pdb/groups/drawable_color.pdb +++ b/pdb/groups/drawable_color.pdb @@ -627,12 +627,16 @@ HELP desc => "Intensity of lowest input" }, { name => 'high_input', type => '0.0 <= float <= 1.0', desc => "Intensity of highest input" }, + { name => 'clamp_input', type => 'boolean', + desc => 'Clamp input values before applying output levels' }, { name => 'gamma', type => '0.1 <= float <= 10', desc => 'Gamma adjustment factor' }, { name => 'low_output', type => '0.0 <= float <= 1.0', desc => "Intensity of lowest output" }, { name => 'high_output', type => '0.0 <= float <= 1.0', - desc => "Intensity of highest output" } + desc => "Intensity of highest output" }, + { name => 'clamp_output', type => 'boolean', + desc => 'Clamp final output values' }, ); %invoke = ( @@ -652,11 +656,13 @@ HELP NULL); g_object_set (config, - "low-input", low_input, - "high-input", high_input, - "gamma", gamma, - "low-output", low_output, - "high-output", high_output, + "low-input", low_input, + "high-input", high_input, + "clamp-input", clamp_input, + "gamma", gamma, + "low-output", low_output, + "high-output", high_output, + "clamp-output", clamp_output, NULL); gimp_drawable_apply_operation_by_name (drawable, progress,