[gimp/gimp-2-10] app: streamline GimpHistogram; avoid spurious channel switch in histogram view
- From: Ell <ell src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gimp/gimp-2-10] app: streamline GimpHistogram; avoid spurious channel switch in histogram view
- Date: Tue, 22 Oct 2019 12:50:35 +0000 (UTC)
commit a2fe44ed8cfc732224b77da765f1ee1dc5d82705
Author: Ell <ell_se yahoo com>
Date: Tue Oct 22 15:37:50 2019 +0300
app: streamline GimpHistogram; avoid spurious channel switch in histogram view
In GimpHistogram, get rid of the "n-channels" property and
corresponding gimp_histogram_n_channels() function. The former
returned the actual number of channels, but this wasn't too useful,
as channel values may not be sequential; the latter returned the
number of components. Instead, add an "n-components" property and
a corresponding gimp_histogram_n_components() function, both of
which return the number of components. Furthermore, add a
gimp_histogram_has_channel() function, which determines if the
histogram has a given channel; this allows for simple testing for
channel availability, which was done wrong in various places.
Adjust the GimpHistogram code for the changes, and clean it up,
fixing a few bugs.
Adjust users of GimpHisotgram for the changes. In particular,
in GimpHisotgramView, fix the channel-availability test when
setting the view's histogram (which happens whenever the active
drawable's preview is frozen), to avoid erroneously swithcing the
view's channel back to "Value" when a non-RGB channel is selected.
(cherry picked from commit fc17f0ed0c404036d923bc15cd3971e955b52dd0)
app/core/gimphistogram.c | 254 ++++++++++++++++++---------------
app/core/gimphistogram.h | 4 +-
app/gimpcore.def | 2 +-
app/operations/gimpoperationequalize.c | 4 +-
app/widgets/gimphistogrameditor.c | 33 +----
app/widgets/gimphistogramview.c | 12 +-
6 files changed, 153 insertions(+), 156 deletions(-)
---
diff --git a/app/core/gimphistogram.c b/app/core/gimphistogram.c
index 6bc23c4c1b..dbf8922865 100644
--- a/app/core/gimphistogram.c
+++ b/app/core/gimphistogram.c
@@ -39,6 +39,9 @@
#include "gimpwaitable.h"
+#define MAX_N_COMPONENTS 4
+#define N_DERIVED_CHANNELS 2
+
#define PIXELS_PER_THREAD \
(/* each thread costs as much as */ 64.0 * 64.0 /* pixels */)
@@ -46,7 +49,7 @@
enum
{
PROP_0,
- PROP_N_CHANNELS,
+ PROP_N_COMPONENTS,
PROP_N_BINS,
PROP_VALUES
};
@@ -87,30 +90,33 @@ typedef struct
/* local function prototypes */
-static void gimp_histogram_finalize (GObject *object);
-static void gimp_histogram_set_property (GObject *object,
- guint property_id,
- const GValue *value,
- GParamSpec *pspec);
-static void gimp_histogram_get_property (GObject *object,
- guint property_id,
- GValue *value,
- GParamSpec *pspec);
+static void gimp_histogram_finalize (GObject *object);
+static void gimp_histogram_set_property (GObject *object,
+ guint property_id,
+ const GValue *value,
+ GParamSpec *pspec);
+static void gimp_histogram_get_property (GObject *object,
+ guint property_id,
+ GValue *value,
+ GParamSpec *pspec);
+
+static gint64 gimp_histogram_get_memsize (GimpObject *object,
+ gint64 *gui_size);
-static gint64 gimp_histogram_get_memsize (GimpObject *object,
- gint64 *gui_size);
+static gboolean gimp_histogram_map_channel (GimpHistogram *histogram,
+ GimpHistogramChannel *channel);
-static void gimp_histogram_set_values (GimpHistogram *histogram,
- gint n_components,
- gint n_bins,
- gdouble *values);
+static void gimp_histogram_set_values (GimpHistogram *histogram,
+ gint n_components,
+ gint n_bins,
+ gdouble *values);
-static void gimp_histogram_calculate_internal (GimpAsync *async,
- CalculateContext *context);
-static void gimp_histogram_calculate_area (const GeglRectangle *area,
- CalculateData *data);
-static void gimp_histogram_calculate_async_callback (GimpAsync *async,
- CalculateContext *context);
+static void gimp_histogram_calculate_internal (GimpAsync *async,
+ CalculateContext *context);
+static void gimp_histogram_calculate_area (const GeglRectangle *area,
+ CalculateData *data);
+static void gimp_histogram_calculate_async_callback (GimpAsync *async,
+ CalculateContext *context);
G_DEFINE_TYPE_WITH_PRIVATE (GimpHistogram, gimp_histogram, GIMP_TYPE_OBJECT)
@@ -130,9 +136,9 @@ gimp_histogram_class_init (GimpHistogramClass *klass)
gimp_object_class->get_memsize = gimp_histogram_get_memsize;
- g_object_class_install_property (object_class, PROP_N_CHANNELS,
- g_param_spec_int ("n-channels", NULL, NULL,
- 0, 6, 0,
+ g_object_class_install_property (object_class, PROP_N_COMPONENTS,
+ g_param_spec_int ("n-components", NULL, NULL,
+ 0, MAX_N_COMPONENTS, 0,
GIMP_PARAM_READABLE));
g_object_class_install_property (object_class, PROP_N_BINS,
@@ -189,8 +195,8 @@ gimp_histogram_get_property (GObject *object,
switch (property_id)
{
- case PROP_N_CHANNELS:
- g_value_set_int (value, histogram->priv->n_channels);
+ case PROP_N_COMPONENTS:
+ g_value_set_int (value, gimp_histogram_n_components (histogram));
break;
case PROP_N_BINS:
@@ -380,7 +386,7 @@ gimp_histogram_clear_values (GimpHistogram *histogram)
{
histogram->priv->n_channels = 0;
- g_object_notify (G_OBJECT (histogram), "n-channels");
+ g_object_notify (G_OBJECT (histogram), "n-components");
}
}
@@ -400,16 +406,11 @@ gimp_histogram_get_maximum (GimpHistogram *histogram,
priv = histogram->priv;
- /* the gray alpha channel is in slot 1 */
- if (priv->n_channels == 4 && channel == GIMP_HISTOGRAM_ALPHA)
- channel = 1;
- /* the luminance channel is in slot 4 */
- else if (priv->n_channels == 5 && channel == GIMP_HISTOGRAM_LUMINANCE)
- channel = 4;
-
if (! priv->values ||
- (channel != GIMP_HISTOGRAM_RGB && channel >= priv->n_channels))
- return 0.0;
+ ! gimp_histogram_map_channel (histogram, &channel))
+ {
+ return 0.0;
+ }
if (channel == GIMP_HISTOGRAM_RGB)
{
@@ -442,18 +443,12 @@ gimp_histogram_get_value (GimpHistogram *histogram,
priv = histogram->priv;
- /* the gray alpha channel is in slot 1 */
- if (priv->n_channels == 4 && channel == GIMP_HISTOGRAM_ALPHA)
- channel = 1;
- /* the luminance channel is in slot 4 */
- else if (priv->n_channels == 5 && channel == GIMP_HISTOGRAM_LUMINANCE)
- channel = 4;
-
- if (! priv->values ||
- bin < 0 || bin >= priv->n_bins ||
- (channel == GIMP_HISTOGRAM_RGB && priv->n_channels < 4) ||
- (channel != GIMP_HISTOGRAM_RGB && channel >= priv->n_channels))
- return 0.0;
+ if (! priv->values ||
+ (bin < 0 || bin >= priv->n_bins) ||
+ ! gimp_histogram_map_channel (histogram, &channel))
+ {
+ return 0.0;
+ }
if (channel == GIMP_HISTOGRAM_RGB)
{
@@ -476,18 +471,18 @@ gimp_histogram_get_component (GimpHistogram *histogram,
{
g_return_val_if_fail (GIMP_IS_HISTOGRAM (histogram), 0.0);
- if (histogram->priv->n_channels > 4)
+ if (gimp_histogram_n_components (histogram) > 2)
component++;
return gimp_histogram_get_value (histogram, component, bin);
}
gint
-gimp_histogram_n_channels (GimpHistogram *histogram)
+gimp_histogram_n_components (GimpHistogram *histogram)
{
g_return_val_if_fail (GIMP_IS_HISTOGRAM (histogram), 0);
- return histogram->priv->n_channels - 2;
+ return histogram->priv->n_channels - N_DERIVED_CHANNELS;
}
gint
@@ -498,6 +493,32 @@ gimp_histogram_n_bins (GimpHistogram *histogram)
return histogram->priv->n_bins;
}
+gboolean
+gimp_histogram_has_channel (GimpHistogram *histogram,
+ GimpHistogramChannel channel)
+{
+ g_return_val_if_fail (GIMP_IS_HISTOGRAM (histogram), FALSE);
+
+ switch (channel)
+ {
+ case GIMP_HISTOGRAM_VALUE:
+ return TRUE;
+
+ case GIMP_HISTOGRAM_RED:
+ case GIMP_HISTOGRAM_GREEN:
+ case GIMP_HISTOGRAM_BLUE:
+ case GIMP_HISTOGRAM_LUMINANCE:
+ case GIMP_HISTOGRAM_RGB:
+ return gimp_histogram_n_components (histogram) >= 3;
+
+ case GIMP_HISTOGRAM_ALPHA:
+ return gimp_histogram_n_components (histogram) == 2 ||
+ gimp_histogram_n_components (histogram) == 4;
+ }
+
+ g_return_val_if_reached (FALSE);
+}
+
gdouble
gimp_histogram_get_count (GimpHistogram *histogram,
GimpHistogramChannel channel,
@@ -512,12 +533,12 @@ gimp_histogram_get_count (GimpHistogram *histogram,
priv = histogram->priv;
- /* the gray alpha channel is in slot 1 */
- if (priv->n_channels == 4 && channel == GIMP_HISTOGRAM_ALPHA)
- channel = 1;
- /* the luminance channel is in slot 4 */
- else if (priv->n_channels == 5 && channel == GIMP_HISTOGRAM_LUMINANCE)
- channel = 4;
+ if (! priv->values ||
+ start > end ||
+ ! gimp_histogram_map_channel (histogram, &channel))
+ {
+ return 0.0;
+ }
if (channel == GIMP_HISTOGRAM_RGB)
return (gimp_histogram_get_count (histogram,
@@ -527,11 +548,6 @@ gimp_histogram_get_count (GimpHistogram *histogram,
gimp_histogram_get_count (histogram,
GIMP_HISTOGRAM_BLUE, start, end));
- if (! priv->values ||
- start > end ||
- channel >= priv->n_channels)
- return 0.0;
-
start = CLAMP (start, 0, priv->n_bins - 1);
end = CLAMP (end, 0, priv->n_bins - 1);
@@ -556,18 +572,12 @@ gimp_histogram_get_mean (GimpHistogram *histogram,
priv = histogram->priv;
- /* the gray alpha channel is in slot 1 */
- if (priv->n_channels == 4 && channel == GIMP_HISTOGRAM_ALPHA)
- channel = 1;
- /* the luminance channel is in slot 4 */
- else if (priv->n_channels == 5 && channel == GIMP_HISTOGRAM_LUMINANCE)
- channel = 4;
-
if (! priv->values ||
- start > end ||
- (channel == GIMP_HISTOGRAM_RGB && priv->n_channels < 5) ||
- (channel != GIMP_HISTOGRAM_RGB && channel >= priv->n_channels))
- return 0.0;
+ start > end ||
+ ! gimp_histogram_map_channel (histogram, &channel))
+ {
+ return 0.0;
+ }
start = CLAMP (start, 0, priv->n_bins - 1);
end = CLAMP (end, 0, priv->n_bins - 1);
@@ -616,18 +626,12 @@ gimp_histogram_get_median (GimpHistogram *histogram,
priv = histogram->priv;
- /* the gray alpha channel is in slot 1 */
- if (priv->n_channels == 4 && channel == GIMP_HISTOGRAM_ALPHA)
- channel = 1;
- /* the luminance channel is in slot 4 */
- else if (priv->n_channels == 5 && channel == GIMP_HISTOGRAM_LUMINANCE)
- channel = 4;
-
if (! priv->values ||
- start > end ||
- (channel == GIMP_HISTOGRAM_RGB && priv->n_channels < 5) ||
- (channel != GIMP_HISTOGRAM_RGB && channel >= priv->n_channels))
- return 0.0;
+ start > end ||
+ ! gimp_histogram_map_channel (histogram, &channel))
+ {
+ return 0.0;
+ }
start = CLAMP (start, 0, priv->n_bins - 1);
end = CLAMP (end, 0, priv->n_bins - 1);
@@ -688,18 +692,12 @@ gimp_histogram_get_threshold (GimpHistogram *histogram,
priv = histogram->priv;
- /* the gray alpha channel is in slot 1 */
- if (priv->n_channels == 4 && channel == GIMP_HISTOGRAM_ALPHA)
- channel = 1;
- /* the luminance channel is in slot 4 */
- else if (priv->n_channels == 5 && channel == GIMP_HISTOGRAM_LUMINANCE)
- channel = 4;
-
if (! priv->values ||
- start > end ||
- (channel == GIMP_HISTOGRAM_RGB && priv->n_channels < 5) ||
- (channel != GIMP_HISTOGRAM_RGB && channel >= priv->n_channels))
- return 0;
+ start > end ||
+ ! gimp_histogram_map_channel (histogram, &channel))
+ {
+ return 0;
+ }
start = CLAMP (start, 0, priv->n_bins - 1);
end = CLAMP (end, 0, priv->n_bins - 1);
@@ -779,18 +777,12 @@ gimp_histogram_get_std_dev (GimpHistogram *histogram,
priv = histogram->priv;
- /* the gray alpha channel is in slot 1 */
- if (priv->n_channels == 4 && channel == GIMP_HISTOGRAM_ALPHA)
- channel = 1;
- /* the luminance channel is in slot 4 */
- else if (priv->n_channels == 5 && channel == GIMP_HISTOGRAM_LUMINANCE)
- channel = 4;
-
if (! priv->values ||
- start > end ||
- (channel == GIMP_HISTOGRAM_RGB && priv->n_channels < 5) ||
- (channel != GIMP_HISTOGRAM_RGB && channel >= priv->n_channels))
- return 0.0;
+ start > end ||
+ ! gimp_histogram_map_channel (histogram, &channel))
+ {
+ return 0.0;
+ }
mean = gimp_histogram_get_mean (histogram, channel, start, end);
count = gimp_histogram_get_count (histogram, channel, start, end);
@@ -822,21 +814,48 @@ gimp_histogram_get_std_dev (GimpHistogram *histogram,
/* private functions */
+static gboolean
+gimp_histogram_map_channel (GimpHistogram *histogram,
+ GimpHistogramChannel *channel)
+{
+ GimpHistogramPrivate *priv = histogram->priv;
+
+ if (*channel == GIMP_HISTOGRAM_RGB)
+ return gimp_histogram_n_components (histogram) >= 3;
+
+ switch (*channel)
+ {
+ case GIMP_HISTOGRAM_ALPHA:
+ if (gimp_histogram_n_components (histogram) == 2)
+ *channel = 1;
+ break;
+
+ case GIMP_HISTOGRAM_LUMINANCE:
+ *channel = gimp_histogram_n_components (histogram) + 1;
+ break;
+
+ default:
+ break;
+ }
+
+ return *channel < priv->n_channels;
+}
+
static void
gimp_histogram_set_values (GimpHistogram *histogram,
gint n_components,
gint n_bins,
gdouble *values)
{
- GimpHistogramPrivate *priv = histogram->priv;
- gboolean notify_n_channels = FALSE;
- gboolean notify_n_bins = FALSE;
+ GimpHistogramPrivate *priv = histogram->priv;
+ gboolean notify_n_components = FALSE;
+ gboolean notify_n_bins = FALSE;
- if (n_components + 2 != priv->n_channels)
+ if (n_components + N_DERIVED_CHANNELS != priv->n_channels)
{
- priv->n_channels = n_components + 2;
+ priv->n_channels = n_components + N_DERIVED_CHANNELS;
- notify_n_channels = TRUE;
+ notify_n_components = TRUE;
}
if (n_bins != priv->n_bins)
@@ -854,8 +873,8 @@ gimp_histogram_set_values (GimpHistogram *histogram,
priv->values = values;
}
- if (notify_n_channels)
- g_object_notify (G_OBJECT (histogram), "n-channels");
+ if (notify_n_components)
+ g_object_notify (G_OBJECT (histogram), "n-components");
if (notify_n_bins)
g_object_notify (G_OBJECT (histogram), "n-bins");
@@ -957,7 +976,8 @@ gimp_histogram_calculate_internal (GimpAsync *async,
if (! async || ! gimp_async_is_canceled (async))
{
gdouble *total_values = NULL;
- gint n_values = (context->n_components + 2) * context->n_bins;
+ gint n_values = (context->n_components + N_DERIVED_CHANNELS) *
+ context->n_bins;
GSList *iter;
for (iter = data.values_list; iter; iter = g_slist_next (iter))
@@ -1014,7 +1034,7 @@ gimp_histogram_calculate_area (const GeglRectangle *area,
n_bins = context->n_bins;
n_components = context->n_components;
- values = g_new0 (gdouble, (n_components + 2) * n_bins);
+ values = g_new0 (gdouble, (n_components + N_DERIVED_CHANNELS) * n_bins);
gimp_atomic_slist_push_head (&data->values_list, values);
iter = gegl_buffer_iterator_new (context->buffer, area, 0,
diff --git a/app/core/gimphistogram.h b/app/core/gimphistogram.h
index 0871ece5b7..2089d82e4e 100644
--- a/app/core/gimphistogram.h
+++ b/app/core/gimphistogram.h
@@ -95,8 +95,10 @@ gdouble gimp_histogram_get_value (GimpHistogram *histogram,
gdouble gimp_histogram_get_component (GimpHistogram *histogram,
gint component,
gint bin);
-gint gimp_histogram_n_channels (GimpHistogram *histogram);
+gint gimp_histogram_n_components (GimpHistogram *histogram);
gint gimp_histogram_n_bins (GimpHistogram *histogram);
+gboolean gimp_histogram_has_channel (GimpHistogram *histogram,
+ GimpHistogramChannel channel);
#endif /* __GIMP_HISTOGRAM_H__ */
diff --git a/app/gimpcore.def b/app/gimpcore.def
index 4459c83d76..3c8801c641 100644
--- a/app/gimpcore.def
+++ b/app/gimpcore.def
@@ -262,7 +262,7 @@ EXPORTS
gimp_histogram_get_median
gimp_histogram_get_std_dev
gimp_histogram_get_value
- gimp_histogram_n_channels
+ gimp_histogram_n_components
gimp_histogram_new
gimp_image_add_channel
gimp_image_add_colormap_entry
diff --git a/app/operations/gimpoperationequalize.c b/app/operations/gimpoperationequalize.c
index b08ca3ee10..119be23b36 100644
--- a/app/operations/gimpoperationequalize.c
+++ b/app/operations/gimpoperationequalize.c
@@ -172,8 +172,8 @@ gimp_operation_equalize_set_property (GObject *object,
pixels = gimp_histogram_get_count (self->histogram,
GIMP_HISTOGRAM_VALUE, 0, n_bins - 1);
- if (gimp_histogram_n_channels (self->histogram) == 1 ||
- gimp_histogram_n_channels (self->histogram) == 2)
+ if (gimp_histogram_n_components (self->histogram) == 1 ||
+ gimp_histogram_n_components (self->histogram) == 2)
max = 1;
else
max = 3;
diff --git a/app/widgets/gimphistogrameditor.c b/app/widgets/gimphistogrameditor.c
index 9c679348d8..0f65f9d1a2 100644
--- a/app/widgets/gimphistogrameditor.c
+++ b/app/widgets/gimphistogrameditor.c
@@ -650,32 +650,6 @@ gimp_histogram_editor_idle_update (GimpHistogramEditor *editor)
return FALSE;
}
-static gboolean
-gimp_histogram_editor_channel_valid (GimpHistogramEditor *editor,
- GimpHistogramChannel channel)
-{
- if (editor->drawable)
- {
- switch (channel)
- {
- case GIMP_HISTOGRAM_VALUE:
- return TRUE;
-
- case GIMP_HISTOGRAM_RED:
- case GIMP_HISTOGRAM_GREEN:
- case GIMP_HISTOGRAM_BLUE:
- case GIMP_HISTOGRAM_LUMINANCE:
- case GIMP_HISTOGRAM_RGB:
- return gimp_drawable_is_rgb (editor->drawable);
-
- case GIMP_HISTOGRAM_ALPHA:
- return gimp_drawable_has_alpha (editor->drawable);
- }
- }
-
- return TRUE;
-}
-
static gboolean
gimp_histogram_menu_sensitivity (gint value,
gpointer data)
@@ -683,8 +657,8 @@ gimp_histogram_menu_sensitivity (gint value,
GimpHistogramEditor *editor = GIMP_HISTOGRAM_EDITOR (data);
GimpHistogramChannel channel = value;
- if (editor->drawable)
- return gimp_histogram_editor_channel_valid (editor, channel);
+ if (editor->histogram)
+ return gimp_histogram_has_channel (editor->histogram, channel);
return FALSE;
}
@@ -696,7 +670,8 @@ gimp_histogram_editor_menu_update (GimpHistogramEditor *editor)
gtk_widget_queue_draw (editor->menu);
- if (! gimp_histogram_editor_channel_valid (editor, view->channel))
+ if (editor->histogram &&
+ ! gimp_histogram_has_channel (editor->histogram, view->channel))
{
gimp_histogram_view_set_channel (view, GIMP_HISTOGRAM_VALUE);
}
diff --git a/app/widgets/gimphistogramview.c b/app/widgets/gimphistogramview.c
index 773e5933f9..bc44dca7f4 100644
--- a/app/widgets/gimphistogramview.c
+++ b/app/widgets/gimphistogramview.c
@@ -619,8 +619,8 @@ gimp_histogram_view_set_histogram (GimpHistogramView *view,
#if 0
g_return_if_fail (histogram == NULL ||
view->bg_histogram == NULL ||
- gimp_histogram_n_channels (view->bg_histogram) ==
- gimp_histogram_n_channels (histogram));
+ gimp_histogram_n_components (view->bg_histogram) ==
+ gimp_histogram_n_components (histogram));
#endif
if (view->histogram != histogram)
@@ -643,7 +643,7 @@ gimp_histogram_view_set_histogram (GimpHistogramView *view,
G_CALLBACK (gimp_histogram_view_notify),
view);
- if (view->channel >= gimp_histogram_n_channels (histogram))
+ if (! gimp_histogram_has_channel (histogram, view->channel))
gimp_histogram_view_set_channel (view, GIMP_HISTOGRAM_VALUE);
}
@@ -669,8 +669,8 @@ gimp_histogram_view_set_background (GimpHistogramView *view,
#if 0
g_return_if_fail (histogram == NULL ||
view->histogram == NULL ||
- gimp_histogram_n_channels (view->histogram) ==
- gimp_histogram_n_channels (histogram));
+ gimp_histogram_n_components (view->histogram) ==
+ gimp_histogram_n_components (histogram));
#endif
if (view->bg_histogram != histogram)
@@ -693,7 +693,7 @@ gimp_histogram_view_set_background (GimpHistogramView *view,
G_CALLBACK (gimp_histogram_view_notify),
view);
- if (view->channel >= gimp_histogram_n_channels (histogram))
+ if (! gimp_histogram_has_channel (histogram, view->channel))
gimp_histogram_view_set_channel (view, GIMP_HISTOGRAM_VALUE);
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]