Unfortunately, there's a little bug in the HSL compose patch, in gimp_hsl_to_rgb4 in the case of zero saturation. This fixes that.

I'd like to have an option to use luminosity rather than value in curves and layers; I'm not sure how to do it without introducing additional complexity.

--- ./libgimpcolor/gimpcolorspace.c~ 2006-06-27 06:33:48.000000000 -0400 +++ ./libgimpcolor/gimpcolorspace.c 2007-01-06 14:57:28.000000000 -0500 @@ -1003,6 +1003,90 @@
}

/**
+ * gimp_rgb_to_hsl4:
+ * @rgb: RGB triplet, rgb[0] is red channel, rgb[1] is green, + * rgb[2] is blue (0..255) + * @hue: Pointer to hue channel (0..1) + * @saturation: Pointer to saturation channel (0..1) + * @luma: Pointer to luma channel (0..1) + *
+ * Convert an RGB color value to an HSL (Hue, Saturation, Lightness) + * color value.
+ *
+ * Since: GIMP 2.4
+ **/
+void
+gimp_rgb_to_hsl4 (const guchar *rgb, + gdouble *hue, + gdouble *saturation, + gdouble *luma) +{
+ gdouble red, green, blue;
+ gdouble h, s, l;
+ gdouble min, max;
+ gdouble delta;
+
+ red = rgb[0] / 255.0;
+ green = rgb[1] / 255.0;
+ blue = rgb[2] / 255.0;
+
+ h = 0.0; /* Shut up -Wall */
+
+ if (red > green)
+ {
+ max = MAX (red, blue);
+ min = MIN (green, blue);
+ }
+ else
+ {
+ max = MAX (green, blue);
+ min = MIN (red, blue);
+ }
+
+ l = (max + min) / 2.0;
+
+ if (max == min)
+ {
+ s = 0.0;
+ h = GIMP_HSL_UNDEFINED;
+ }
+ else
+ {
+ if (l < 0.0)
+ h += 1.0;
+ }
+
+ *hue = h;
+ *saturation = s;
+ *luma = l;
+}
+
+/**
* gimp_hsv_to_rgb4:
* @rgb: RGB triplet, rgb[0] is red channel, rgb[1] is green, * rgb[2] is blue (0..255) @@ -1083,3 +1167,45 @@
rgb[1] = ROUND (saturation * 255.0); rgb[2] = ROUND (value * 255.0); }
+
+/**
+ * gimp_hsl_to_rgb4:
+ * @rgb: RGB triplet, rgb[0] is red channel, rgb[1] is green, + * rgb[2] is blue (0..255) + * @hue: Hue channel (0..1)
+ * @saturation: Saturation channel (0..1) + * @luma: Luma channel (0..1) + *
+ * Convert an HSL color value (Hue, Saturation, Lightness) to an RGB + * color value.
+ *
+ * Since: GIMP 2.4
+ **/
+void
+gimp_hsl_to_rgb4 (guchar *rgb,
+ gdouble hue,
+ gdouble saturation, + gdouble luma)
+{
+ if (saturation == 0.0)
+ {
+ rgb[0] = ROUND(luma * 255.0); + rgb[1] = ROUND(luma * 255.0); + rgb[2] = ROUND(luma * 255.0); + }
+ else
+ {
+ gdouble m1, m2;
+
+ if (luma 0)
+ {
+ gimp_rgb_to_hsl4 (rgb_src, &hue, &sat, &val); + *hue_dst++ = (guchar) (hue * 255.999); + *sat_dst++ = (guchar) (sat * 255.999); + *lum_dst++ = (guchar) (val * 255.999); + rgb_src += offset;
+ }
+}
+
+
+static void
+extract_huel (const guchar *src,
+ gint bpp,
+ gint numpix,
+ guchar **dst)
+{
+ register const guchar *rgb_src = src; + register guchar *hue_dst = dst[0]; + register gint count = numpix, offset = bpp; + gdouble hue, dummy;
+
+ while (count-- > 0)
+ {
+ gimp_rgb_to_hsl4 (rgb_src, &hue, &dummy, &dummy); + *hue_dst++ = (guchar) (hue * 255.999); + rgb_src += offset;
+ }
+}
+
+
+static void
+extract_satl (const guchar *src,
+ gint bpp,
+ gint numpix,
+ guchar **dst)
+{
+ register const guchar *rgb_src = src; + register guchar *sat_dst = dst[0]; + register gint count = numpix, offset = bpp; + gdouble sat, dummy;
+
+ while (count-- > 0)
+ {
+ gimp_rgb_to_hsl4 (rgb_src, &dummy, &sat, &dummy); + *sat_dst++ = (guchar) (sat * 255.999); + rgb_src += offset;
+ }
+}
+
+
+static void
+extract_luma (const guchar *src,
+ gint bpp,
+ gint numpix,
+ guchar **dst)
+{
+ register const guchar *rgb_src = src; + register guchar *lum_dst = dst[0]; + register gint count = numpix, offset = bpp; + gdouble lum, dummy;
+
+ while (count-- > 0)
+ {
+ gimp_rgb_to_hsl4 (rgb_src, &dummy, &dummy, &lum); + *lum_dst++ = (guchar) (lum * 255.999); + rgb_src += offset;
+ }
+}
+
+
+static void
extract_cyan (const guchar *src,
gint bpp,
gint numpix,
--- ./plug-ins/common/compose.c~ 2006-09-01 07:14:34.000000000 -0400 +++ ./plug-ins/common/compose.c 2007-01-06 13:08:52.000000000 -0500 @@ -93,6 +93,11 @@
gint numpix, guchar *dst, gboolean dst_has_alpha); +static void compose_hsl (guchar **src, + gint *incr, + gint numpix, + guchar *dst, + gboolean dst_has_alpha); static void compose_cmy (guchar **src, gint *incr, gint numpix, @@ -211,6 +216,14 @@
{ NULL, NULL, NULL, NULL },
"hsv-compose", compose_hsv },

+ { N_("HSL"), 3,
+ { N_("_Hue:"),
+ N_("_Saturation:"),
+ N_("_Luma:"),
+ NULL },
+ { NULL, NULL, NULL, NULL },
+ "hsl-compose", compose_hsl },
+
{ N_("CMY"), 3,
{ N_("_Cyan:"),
N_("_Magenta:"),
@@ -337,7 +350,7 @@
{ GIMP_PDB_IMAGE, "image2", "Second input image" }, { GIMP_PDB_IMAGE, "image3", "Third input image" }, { GIMP_PDB_IMAGE, "image4", "Fourth input image" }, - { GIMP_PDB_STRING, "compose-type", "What to compose: RGB, RGBA, HSV, CMY, CMYK" }, + { GIMP_PDB_STRING, "compose-type", "What to compose: RGB, RGBA, HSV, HSL, CMY, CMYK" }, { GIMP_PDB_INT8, "value1", "Mask value if image 1 is -1" }, { GIMP_PDB_INT8, "value2", "Mask value if image 2 is -1" }, { GIMP_PDB_INT8, "value3", "Mask value if image 3 is -1" }, @@ -357,7 +370,7 @@
{ GIMP_PDB_DRAWABLE, "drawable2", "Second input drawable" }, { GIMP_PDB_DRAWABLE, "drawable3", "Third input drawable" }, { GIMP_PDB_DRAWABLE, "drawable4", "Fourth input drawable" }, - { GIMP_PDB_STRING, "compose-type", "What to compose: RGB, RGBA, HSV, CMY, CMYK" }, + { GIMP_PDB_STRING, "compose-type", "What to compose: RGB, RGBA, HSV, HSL, CMY, CMYK" }, { GIMP_PDB_INT8, "value1", "Mask value if image 1 is -1" }, { GIMP_PDB_INT8, "value2", "Mask value if image 2 is -1" }, { GIMP_PDB_INT8, "value3", "Mask value if image 3 is -1" }, @@ -1035,6 +1048,38 @@


static void
+compose_hsl (guchar **src,
+ gint *incr_src,
+ gint numpix,
+ guchar *dst,
+ gboolean dst_has_alpha) +{
+ register const guchar *hue_src = src[0]; + register const guchar *sat_src = src[1]; + register const guchar *lum_src = src[2]; + register guchar *rgb_dst = dst; + register gint count = numpix; + gint hue_incr = incr_src[0];
+ gint sat_incr = incr_src[1];
+ gint lum_incr = incr_src[2];
+
+ while (count-- > 0)
+ {
+ gimp_hsl_to_rgb4 (rgb_dst, (gdouble) *hue_src / 255.0, + (gdouble) *sat_src / 255.0, + (gdouble) *lum_src / 255.0); + rgb_dst += 3;
+ hue_src += hue_incr;
+ sat_src += sat_incr;
+ lum_src += lum_incr;
+
+ if (dst_has_alpha)
+ rgb_dst++;
+ }
+}
+
+
+static void
compose_cmy (guchar **src,
gint *incr_src,
gint numpix,

From: Sven Neumann
Date: Wed, 10 Jan 2007 08:53:02 +0100

can you explain to me why your patch introduces new functions in libgimpcolor? Couldn't you just use the existing gimp_rgb_to_hsl() and gimp_hsl_to_rgb()?

I did it the way I did to use the same implementation as the existing compose_hsv() function. In addition, it isn't clear to me why there should be gimp_rgb_to_hsv4 but not gimp_rgb_to_hsl4.

From: Sven Neumann
Date: Thu, 11 Jan 2007 08:22:20 +0100

On Wed, 2007-01-10 at 06:53 -0500, Robert L Krawitz wrote: > From: Sven Neumann
> Date: Wed, 10 Jan 2007 08:53:02 +0100 >
> can you explain to me why your patch introduces new functions in > libgimpcolor? Couldn't you just use the existing gimp_rgb_to_hsl() > and gimp_hsl_to_rgb()?
>
> I did it the way I did to use the same implementation as the existing > compose_hsv() function. In addition, it isn't clear to me why there > should be gimp_rgb_to_hsv4 but not gimp_rgb_to_hsl4.

gimp_rgb_to_hsv4() is there for historical reasons. If we could, we would probably remove it from the API. I don't think we want to introduce a HSL variant of it now.

I'll try to get to it tonight, although I think it would be useful to have the floating point version also -- if someone's writing a plugin that's doing multiple transformations, it keeps more precision to do the math in floating point vs. with chars. In this case, I suggest that the other floating point variants should be marked deprecated.

From: Sven Neumann
Date: Thu, 11 Jan 2007 18:55:44 +0100

On Thu, 2007-01-11 at 07:23 -0500, Robert L Krawitz wrote:

> I'll try to get to it tonight, although I think it would be > useful to have the floating point version also -- if someone's > writing a plugin that's doing multiple transformations, it keeps > more precision to do the math in floating point vs. with chars. > In this case, I suggest that the other floating point variants > should be marked deprecated.

But gimp_rgb_to_hsl() and gimp_hsl_to_rgb() are floating point versions. What's your point?

Uh, maybe that I didn't look closely enough :-? Here's an updated patch.

BTW, in the process of preparing this diff, I notice that the tarball contains devel-docs/app/Makefile, which almost surely it shouldn't.

diff -ru src/gimp-2.3.13/plug-ins/common/compose.c sandbox/gimp-2.3.13/plug-ins/common/compose.c --- src/gimp-2.3.13/plug-ins/common/compose.c 2006-09-01 07:14:34.000000000 -0400 +++ sandbox/gimp-2.3.13/plug-ins/common/compose.c 2007-01-11 20:56:22.000000000 -0500 @@ -93,6 +93,11 @@
gint numpix, guchar *dst, gboolean dst_has_alpha); +static void compose_hsl (guchar **src, + gint *incr, + gint numpix, + guchar *dst, + gboolean dst_has_alpha); static void compose_cmy (guchar **src, gint *incr, gint numpix, @@ -211,6 +216,14 @@
{ NULL, NULL, NULL, NULL },
"hsv-compose", compose_hsv },

+ { N_("HSL"), 3,
+ { N_("_Hue:"),
+ N_("_Saturation:"),
+ N_("_Lightness:"),
+ NULL },
+ { NULL, NULL, NULL, NULL },
+ "hsl-compose", compose_hsl },
+
{ N_("CMY"), 3,
{ N_("_Cyan:"),
N_("_Magenta:"),
@@ -337,7 +350,7 @@
{ GIMP_PDB_IMAGE, "image2", "Second input image" }, { GIMP_PDB_IMAGE, "image3", "Third input image" }, { GIMP_PDB_IMAGE, "image4", "Fourth input image" }, - { GIMP_PDB_STRING, "compose-type", "What to compose: RGB, RGBA, HSV, CMY, CMYK" }, + { GIMP_PDB_STRING, "compose-type", "What to compose: RGB, RGBA, HSV, HSL, CMY, CMYK" }, { GIMP_PDB_INT8, "value1", "Mask value if image 1 is -1" }, { GIMP_PDB_INT8, "value2", "Mask value if image 2 is -1" }, { GIMP_PDB_INT8, "value3", "Mask value if image 3 is -1" }, @@ -357,7 +370,7 @@
{ GIMP_PDB_DRAWABLE, "drawable2", "Second input drawable" }, { GIMP_PDB_DRAWABLE, "drawable3", "Third input drawable" }, { GIMP_PDB_DRAWABLE, "drawable4", "Fourth input drawable" }, - { GIMP_PDB_STRING, "compose-type", "What to compose: RGB, RGBA, HSV, CMY, CMYK" }, + { GIMP_PDB_STRING, "compose-type", "What to compose: RGB, RGBA, HSV, HSL, CMY, CMYK" }, { GIMP_PDB_INT8, "value1", "Mask value if image 1 is -1" }, { GIMP_PDB_INT8, "value2", "Mask value if image 2 is -1" }, { GIMP_PDB_INT8, "value3", "Mask value if image 3 is -1" }, @@ -1035,6 +1048,44 @@


static void
+compose_hsl (guchar **src,
+ gint *incr_src,
+ gint numpix,
+ guchar *dst,
+ gboolean dst_has_alpha) +{
+ register const guchar *hue_src = src[0]; + register const guchar *sat_src = src[1]; + register const guchar *lum_src = src[2]; + register guchar *rgb_dst = dst; + register gint count = numpix; + gint hue_incr = incr_src[0];
+ gint sat_incr = incr_src[1];
+ gint lum_incr = incr_src[2];
+ GimpHSL hsl;
+ GimpRGB rgb;
+
+ while (count-- > 0)
+ {
+ hsl.h = (gdouble) *hue_src / 255.0; + hsl.s = (gdouble) *sat_src / 255.0; + hsl.l = (gdouble) *lum_src / 255.0; + gimp_hsl_to_rgb (&hsl, &rgb); + rgb_dst[0] = (guchar) (rgb.r * 255.999); + rgb_dst[1] = (guchar) (rgb.g * 255.999); + rgb_dst[2] = (guchar) (rgb.b * 255.999); + rgb_dst += 3;
+ hue_src += hue_incr;
+ sat_src += sat_incr;
+ lum_src += lum_incr;
+
+ if (dst_has_alpha)
+ rgb_dst++;
+ }
+}
+
+
+static void
compose_cmy (guchar **src,
gint *incr_src,
gint numpix,
diff -ru src/gimp-2.3.13/plug-ins/common/decompose.c sandbox/gimp-2.3.13/plug-ins/common/decompose.c --- src/gimp-2.3.13/plug-ins/common/decompose.c 2006-06-27 06:33:49.000000000 -0400 +++ sandbox/gimp-2.3.13/plug-ins/common/decompose.c 2007-01-11 20:55:51.000000000 -0500 @@ -100,12 +100,20 @@
gint bpp, gint numpix, guchar **dst); static void extract_hsv (const guchar *src, gint bpp, gint numpix, guchar **dst); +static void extract_hsl (const guchar *src, + gint bpp, gint numpix, guchar **dst); static void extract_hue (const guchar *src, gint bpp, gint numpix, guchar **dst); static void extract_sat (const guchar *src, gint bpp, gint numpix, guchar **dst); static void extract_val (const guchar *src, gint bpp, gint numpix, guchar **dst); +static void extract_huel (const guchar *src, + gint bpp, gint numpix, guchar **dst); +static void extract_satl (const guchar *src, + gint bpp, gint numpix, guchar **dst); +static void extract_lightness (const guchar *src, + gint bpp, gint numpix, guchar **dst); static void extract_cmy (const guchar *src, gint bpp, gint numpix, guchar **dst); static void extract_cyan (const guchar *src, @@ -182,6 +190,15 @@
{ N_("Value"), FALSE, 1, { N_("value") }, extract_val },

+ { N_("HSL"), TRUE, 3, { N_("hue_l"), + N_("saturation_l"), + N_("lightness") }, extract_hsl }, +
+ { N_("Hue (HSL)"), FALSE, 1, { N_("hue_l") }, extract_huel }, + { N_("Saturation (HSL)"), FALSE, 1, { N_("saturation_l") }, extract_satl }, + { N_("Lightness"), FALSE, 1, { N_("lightness")}, extract_lightness }, +
+
{ N_("CMY"), TRUE, 3, { N_("cyan"), N_("magenta"), N_("yellow") }, extract_cmy }, @@ -957,6 +974,106 @@


static void
+extract_hsl (const guchar *src,
+ gint bpp,
+ gint numpix,
+ guchar **dst)
+{
+ register const guchar *rgb_src = src; + register guchar *hue_dst = dst[0]; + register guchar *sat_dst = dst[1]; + register guchar *lum_dst = dst[2]; + register gint count = numpix, offset = bpp; + GimpRGB rgb;
+ GimpHSL hsl;
+
+ while (count-- > 0)
+ {
+ rgb.r = (gdouble) rgb_src[0] / 255.0; + rgb.g = (gdouble) rgb_src[1] / 255.0; + rgb.b = (gdouble) rgb_src[2] / 255.0; + gimp_rgb_to_hsl (&rgb, &hsl); + *hue_dst++ = (guchar) (hsl.h * 255.999); + *sat_dst++ = (guchar) (hsl.s * 255.999); + *lum_dst++ = (guchar) (hsl.l * 255.999); + rgb_src += offset;
+ }
+}
+
+
+static void
+extract_huel (const guchar *src,
+ gint bpp,
+ gint numpix,
+ guchar **dst)
+{
+ register const guchar *rgb_src = src; + register guchar *hue_dst = dst[0]; + register gint count = numpix, offset = bpp; + GimpRGB rgb;
+ GimpHSL hsl;
+
+ while (count-- > 0)
+ {
+ rgb.r = (gdouble) rgb_src[0] / 255.0; + rgb.g = (gdouble) rgb_src[1] / 255.0; + rgb.b = (gdouble) rgb_src[2] / 255.0; + gimp_rgb_to_hsl (&rgb, &hsl); + *hue_dst++ = (guchar) (hsl.h * 255.999); + rgb_src += offset;
+ }
+}
+
+
+static void
+extract_satl (const guchar *src,
+ gint bpp,
+ gint numpix,
+ guchar **dst)
+{
+ register const guchar *rgb_src = src; + register guchar *sat_dst = dst[0]; + register gint count = numpix, offset = bpp; + GimpRGB rgb;
+ GimpHSL hsl;
+
+ while (count-- > 0)
+ {
+ rgb.r = (gdouble) rgb_src[0] / 255.0; + rgb.g = (gdouble) rgb_src[1] / 255.0; + rgb.b = (gdouble) rgb_src[2] / 255.0; + gimp_rgb_to_hsl (&rgb, &hsl); + *sat_dst++ = (guchar) (hsl.s * 255.999); + rgb_src += offset;
+ }
+}
+
+
+static void
+extract_lightness (const guchar *src, + gint bpp,
+ gint numpix,
+ guchar **dst)
+{
+ register const guchar *rgb_src = src; + register guchar *lum_dst = dst[0]; + register gint count = numpix, offset = bpp; + GimpRGB rgb;
+ GimpHSL hsl;
+
+ while (count-- > 0)
+ {
+ rgb.r = (gdouble) rgb_src[0] / 255.0; + rgb.g = (gdouble) rgb_src[1] / 255.0; + rgb.b = (gdouble) rgb_src[2] / 255.0; + gimp_rgb_to_hsl (&rgb, &hsl); + *lum_dst++ = (guchar) (hsl.l * 255.999); + rgb_src += offset;
+ }
+}
+
+
+static void
extract_cyan (const guchar *src,
gint bpp,
gint numpix,

From: Sven Neumann
Date: Fri, 12 Jan 2007 21:33:01 +0100

On Thu, 2007-01-11 at 21:10 -0500, Robert L Krawitz wrote:

> Uh, maybe that I didn't look closely enough :-? Here's an updated > patch.

Thanks. I think it's about time that you open a bug report for this new feature and attach your patch to it. That's still the preferred way of submitting patches.

Done (bug 395928).