From a040dfd6e4e5f1dda535a87cf0463ed80c2f835c Mon Sep 17 00:00:00 2001 From: Jehan Date: Thu, 10 Oct 2024 03:12:34 +0200 Subject: [PATCH] Issue #11339: color selection sliders don't always update for slow increments. Rather than the previously reverted commit, the proper solution is: * gimp_color_selector_set_color() must not test for perceptual identity because GimpColorSelector is too much of a generic class. In some case, such a test may be worth it to limit costly updates (in particular when it implies some rendering of color surfaces), but this would happen in specific subclasses. * In GimpColorSelection, the GimpColorScales show numbers, so any change in them will likely trigger other scales to change as a side effect. Therefore when handling the "color-changed" signal on these scales, however small the change may be, we want to run the update. Now removing this test in gimp_color_selector_set_color() also revealed a serious bug which I fix in this commit, which is that the binding between the "value" of a GimpLabelSpin with the "value" of its adjustment was still triggering repeated property-setting, which was enough to freeze the GUI for a while. The logic of using only the GtkAdjustment's value as a source while also binding both properties was not robust enough. Instead the GimpLabelSpin will now store its own value and the binding will simply keep it in sync with the one in the adjustment. Note that this is also part of the solution for #10998, because it means there were cases where the color displayed in scales of the color selection dialog was not actually the color set as foreground or background. --- libgimpwidgets/gimpcolorselection.c | 14 ++++---------- libgimpwidgets/gimpcolorselector.c | 14 ++++---------- libgimpwidgets/gimplabelspin.c | 16 ++++++++-------- 3 files changed, 16 insertions(+), 28 deletions(-) diff --git a/libgimpwidgets/gimpcolorselection.c b/libgimpwidgets/gimpcolorselection.c index 2383265a5a..8c7c01bb75 100644 --- a/libgimpwidgets/gimpcolorselection.c +++ b/libgimpwidgets/gimpcolorselection.c @@ -753,21 +753,15 @@ gimp_color_selection_scales_changed (GimpColorSelector *selector, GimpColorSelection *selection) { GimpColorSelectionPrivate *priv; - GeglColor *old_color; priv = gimp_color_selection_get_instance_private (selection); - old_color = priv->color; + g_object_unref (priv->color); priv->color = gegl_color_duplicate (color); - if (! gimp_color_is_perceptually_identical (priv->color, old_color)) - { - gimp_color_selection_update (selection, - UPDATE_ENTRY | UPDATE_NOTEBOOK | UPDATE_COLOR); - gimp_color_selection_color_changed (selection); - } - - g_object_unref (old_color); + gimp_color_selection_update (selection, + UPDATE_ENTRY | UPDATE_NOTEBOOK | UPDATE_COLOR); + gimp_color_selection_color_changed (selection); } static void diff --git a/libgimpwidgets/gimpcolorselector.c b/libgimpwidgets/gimpcolorselector.c index c32fc08770..6286bc7a0f 100644 --- a/libgimpwidgets/gimpcolorselector.c +++ b/libgimpwidgets/gimpcolorselector.c @@ -407,27 +407,21 @@ gimp_color_selector_set_color (GimpColorSelector *selector, { GimpColorSelectorClass *selector_class; GimpColorSelectorPrivate *priv; - GeglColor *old_color; g_return_if_fail (GIMP_IS_COLOR_SELECTOR (selector)); g_return_if_fail (GEGL_IS_COLOR (color)); priv = gimp_color_selector_get_instance_private (selector); - old_color = priv->color; + g_object_unref (priv->color); priv->color = gegl_color_duplicate (color); selector_class = GIMP_COLOR_SELECTOR_GET_CLASS (selector); - if (! gimp_color_is_perceptually_identical (priv->color, old_color)) - { - if (selector_class->set_color) - selector_class->set_color (selector, priv->color); + if (selector_class->set_color) + selector_class->set_color (selector, priv->color); - gimp_color_selector_emit_color_changed (selector); - } - - g_object_unref (old_color); + gimp_color_selector_emit_color_changed (selector); } /** diff --git a/libgimpwidgets/gimplabelspin.c b/libgimpwidgets/gimplabelspin.c index e94cd9c7ba..fa5be40cfd 100644 --- a/libgimpwidgets/gimplabelspin.c +++ b/libgimpwidgets/gimplabelspin.c @@ -62,6 +62,8 @@ typedef struct _GimpLabelSpinPrivate GtkAdjustment *spin_adjustment; gint digits; + + gdouble value; } GimpLabelSpinPrivate; static void gimp_label_spin_constructed (GObject *object); @@ -216,13 +218,11 @@ gimp_label_spin_set_property (GObject *object, switch (property_id) { case PROP_VALUE: - /* Avoid looping forever since we have bound this widget's - * "value" property with the spin button "value" property. - */ - if (gtk_adjustment_get_value (priv->spin_adjustment) != g_value_get_double (value)) - gtk_adjustment_set_value (priv->spin_adjustment, g_value_get_double (value)); - - g_signal_emit (object, gimp_label_spin_signals[VALUE_CHANGED], 0); + if (priv->value != g_value_get_double (value)) + { + priv->value = g_value_get_double (value); + g_signal_emit (object, gimp_label_spin_signals[VALUE_CHANGED], 0); + } break; case PROP_LOWER: gtk_adjustment_set_lower (priv->spin_adjustment, @@ -267,7 +267,7 @@ gimp_label_spin_get_property (GObject *object, switch (property_id) { case PROP_VALUE: - g_value_set_double (value, gtk_adjustment_get_value (priv->spin_adjustment)); + g_value_set_double (value, priv->value); break; case PROP_LOWER: g_value_set_double (value, gtk_adjustment_get_lower (priv->spin_adjustment));