[PATCH] color-hsv-to-rgb

[PATCH] color-hsv-to-rgb

[Dov Grobgeld]

[-- Attachment #1.1: Type: text/plain, Size: 246 bytes --]

Hello.

I would like to do my first(!) contribution to emacs with the attached
patch. Is there interest, or is it too trivial? Do I need to do a copyright
assignment? Is there a guide for dummies on how to do that?

With hope for better times...

[-- Attachment #1.2: Type: text/html, Size: 842 bytes --]

[-- Attachment #2: color-hsv-to-rgb.patch --]
[-- Type: text/x-patch, Size: 1110 bytes --]

diff --git a/lisp/color.el b/lisp/color.el
index f68cf5e6b17..37f11f23c7c 100644
--- a/lisp/color.el
+++ b/lisp/color.el
@@ -123,6 +123,26 @@ color-complement-hex
   "Return the color that is the complement of COLOR, in hexadecimal format."
   (apply 'color-rgb-to-hex (color-complement color)))
 
+(defun color-hsv-to-rgb (H S V)
+  "Convert hue, saturation and value to their RGB representation.
+H, S, and V should each be numbers between 0.0 and 1.0, inclusive.
+Return a list (RED GREEN BLUE), where each element is between 0.0 and 1.0,
+inclusive."
+  (let* ((I (floor (* 6 H)))
+         (F (- (* H 6) I))
+         (P (* V (- 1.0 S)))
+         (Q (* V (- 1.0 (* F S))))
+         (T (* V (- 1.0 (* (- 1.0 F) S))))
+         (I (mod I 6)))
+    (cond
+     ((= I 0) (list V T P))
+     ((= I 1) (list Q V P))
+     ((= I 2) (list P V T))
+     ((= I 3) (list P Q V))
+     ((= I 4) (list T P V))
+     ((= I 5) (list V P Q))
+     (t (list 0 0 0)))))
+
 (defun color-rgb-to-hsv (red green blue)
   "Convert RGB color components to HSV.
 RED, GREEN, and BLUE should each be numbers between 0.0 and 1.0,

Re: [PATCH] color-hsv-to-rgb

[Stefan Kangas]

Dov Grobgeld <dov.grobgeld@gmail.com> writes:

> I would like to do my first(!) contribution to emacs with the attached
> patch. Is there interest, or is it too trivial? Do I need to do a copyright
> assignment? Is there a guide for dummies on how to do that?

Welcome to Emacs!

I don't see why we couldn't take a patch implementing the inverse of
`color-rgb-to-hsv', so I think there's interest.  It would be
interesting to know what you need it for, though.

Your patch is above 15 lines, so we would need a copyright assignment
for it.  See the form below.  When the assignment is done, please send
your patch to bug-gnu-emacs@gnu.org, which will record it in our bug
tracker.  We risk losing track of patches sent to emacs-devel.

Thanks again for your interest in Emacs.

---

Please email the following information to assign@gnu.org, and we
will send you the assignment form for your past and future changes.

Please use your full legal name (in ASCII characters) as the subject
line of the message.
----------------------------------------------------------------------
REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES

[What is the name of the program or package you're contributing to?]
Emacs

[Did you copy any files or text written by someone else in these changes?
Even if that material is free software, we need to know about it.]

[Do you have an employer who might have a basis to claim to own
your changes?  Do you attend a school which might make such a claim?]

[For the copyright registration, what country are you a citizen of?]

[What year were you born?]

[Please write your email address here.]

[Please write your postal address here.]

[Which files have you changed so far, and which new files have you written
so far?]

Re: [PATCH] color-hsv-to-rgb

[Jim Porter]

On 10/16/2023 12:59 AM, Stefan Kangas wrote:
> Dov Grobgeld <dov.grobgeld@gmail.com> writes:
> 
>> I would like to do my first(!) contribution to emacs with the attached
>> patch. Is there interest, or is it too trivial? Do I need to do a copyright
>> assignment? Is there a guide for dummies on how to do that?
> 
> Welcome to Emacs!
> 
> I don't see why we couldn't take a patch implementing the inverse of
> `color-rgb-to-hsv', so I think there's interest.  It would be
> interesting to know what you need it for, though.

For what it's worth, I thought about adding this function too. I would 
use it for a custom theme to choose colors for 'vc-annotate'. My 
hypothetical code would take a starting color (in RGB), convert to HSV, 
and then increment the value component to produce gradually lighter 
colors. After that, I'd need a way to convert back to RGB to make the 
hex string for the face attribute, hence this new function.

(Even better would be conversions between RGB and LCh, but that's a 
bigger project.)

RE: [External] : Re: [PATCH] color-hsv-to-rgb

[Drew Adams]

> For what it's worth, I thought about adding this function too. I would
> use it for a custom theme to choose colors for 'vc-annotate'. My
> hypothetical code would take a starting color (in RGB), convert to HSV,
> and then increment the value component to produce gradually lighter
> colors. After that, I'd need a way to convert back to RGB to make the
> hex string for the face attribute, hence this new function.

Yes.  It should have been added long ago.

It's been available in hexrgb.el since 2005.

And hexrgb.el was offered to GNU Emacs.
And color.el was derived from hexrgb.el.

https://www.emacswiki.org/emacs/download/hexrgb.el

And you don't need to go to RGB to get
the hex RGB representation from HSV:

hexrgb-hsv-to-hex is a compiled Lisp function in ‘hexrgb.el’.

(hexrgb-hsv-to-hex HUE SATURATION VALUE &optional NB-DIGITS)

Return the hex RBG color string for inputs HUE, SATURATION, VALUE.
Those inputs are each in the range 0.0 to 1.0, inclusive.

Optional arg NB-DIGITS is the number of hex digits per component.  It
should be 1, 2, 3, or 4 (default: 4).

The output string is `#' followed by NB-DIGITS hex digits for each
color component.  So for the default NB-DIGITS value of 4, the form is
"#RRRRGGGGBBBB".

RE: [External] : Re: [PATCH] color-hsv-to-rgb

[Stefan Kangas]

Drew Adams <drew.adams@oracle.com> writes:

> And hexrgb.el was offered to GNU Emacs.
> And color.el was derived from hexrgb.el.
>
> https://www.emacswiki.org/emacs/download/hexrgb.el

What are the main things besides `color-hsv-to-rgb' that hexrgb.el has
but color.el doesn't, in your view?

RE: [External] : Re: [PATCH] color-hsv-to-rgb

[Drew Adams]

> What are the main things besides `color-hsv-to-rgb' that hexrgb.el has
> but color.el doesn't, in your view?

Sorry, but I don't know and I don't care.
If you're interested you can take a look.

I was there when color.el was created.
Things like hexrgb-hsv-to-rgb were no
doubt considered by Someone (TM) then.

There's not a lot in  hexrgb.el now that
wasn't in it back then.  But hsv-to-rgb
was one of the first things included.

Re: [External] : Re: [PATCH] color-hsv-to-rgb

[Eli Zaretskii]

> From: Drew Adams <drew.adams@oracle.com>
> Date: Thu, 19 Oct 2023 21:35:42 +0000
> 
> > What are the main things besides `color-hsv-to-rgb' that hexrgb.el has
> > but color.el doesn't, in your view?
> 
> Sorry, but I don't know and I don't care.

That's just rude.

RE: [External] : Re: [PATCH] color-hsv-to-rgb

[Drew Adams]

> > > What are the main things besides
> > > `color-hsv-to-rgb' that hexrgb.el has
> > > but color.el doesn't, in your view?
> >
> > Sorry, but I don't know and I don't care.
> 
> That's just rude.

It's not rude at all.

 1. I don't know.
 2. I don't care.
 3. Sorry I'm not any help with that.

My post was not to solicit inclusion of
anything particular from hexrgb.el.

I pointed out that (1) `hexrgb-hsv-to-rgb'
was provided (to Emacs) _long ago_, (2) I
agree that such a function is useful for
Emacs (still), and (3) code for it is in
hexrgb.el, for use as is or as food for
thought.

Wrt #2, I seconded the second person who
said that such a function could be useful.

If anyone is interested in finding what
else might be of interest for Emacs in
hexrgb.el, they are _welcome_ to take a
look.  The file isn't large, the source
code is accessible and clear, I've signed
FSF papers, and I give my permission if
any of hexrgb.el is to be used.

That's all.

I'm not interested in getting into a
discussion or arguments about what might
or might not be useful from hexrgb.el
for Emacs.  It's there - _FYI_.

Look inside it or don't; I really don't
care.  Should I?  Is it rude not to care,
and to let you know straightforwardly
that I don't?

Is it hard, for any who really want to
know, to look inside the file and decide
for themselves?  I don't have an opinion
about what they might want.

I was asked for "my view", and the answer
is that I don't have a view about what
else might be of interest to Emacs.  The
last time I looked inside the file was
likely 2018 - the last time I updated it.
(Naturally, if someone has a technical
question then I'll take a look and try to
answer it.)
___

What might reasonably be considered rude
is turning the discussion into an ad
hominem attack by calling me rude.  But
that's your specialty (weakness).

My mails about this, including the one
where I said I don't know and don't care,
have been intended to help.  If you want
to find out what might be of interest to
you in hexrgb.el then peek inside it.
I don't make any claim that anything in
there is worth your peeking.  I really
don't care whether you do.

Please don't bother to try to find some
phrase in this mail to quote alone, out
of context, and continue your ad hominem
nonsense.  No one needs that.  Thx.

If you have something technical to say
to me about (color|hexrgb)-hsv-to-rgb or
hexrgb.el, then that's welcome.

Re: [External] : Re: [PATCH] color-hsv-to-rgb

[Eli Zaretskii]

> From: Drew Adams <drew.adams@oracle.com>
> CC: "stefankangas@gmail.com" <stefankangas@gmail.com>,
>         "jporterbugs@gmail.com"
> 	<jporterbugs@gmail.com>,
>         "dov.grobgeld@gmail.com" <dov.grobgeld@gmail.com>,
>         "emacs-devel@gnu.org" <emacs-devel@gnu.org>
> Date: Fri, 20 Oct 2023 15:04:48 +0000
> 
> > > > What are the main things besides
> > > > `color-hsv-to-rgb' that hexrgb.el has
> > > > but color.el doesn't, in your view?
> > >
> > > Sorry, but I don't know and I don't care.
> > 
> > That's just rude.
> 
> It's not rude at all.

You were kindly asked to help Emacs developers decide which parts of
hexrgb.el could be useful for Emacs.  You replied that you didn't
care.  If that is not rudeness, then I don't know what rudeness is.

> If anyone is interested in finding what
> else might be of interest for Emacs in
> hexrgb.el, they are _welcome_ to take a
> look.  The file isn't large, the source
> code is accessible and clear, I've signed
> FSF papers, and I give my permission if
> any of hexrgb.el is to be used.

You were asked to provide your POV on this.  Why would you reply to a
reasonable request in such a rude way is anyone's guess.

> Look inside it or don't; I really don't
> care.  Should I?

You should, if you care about Emacs.

> Is it rude not to care, and to let you know straightforwardly that I
> don't?

It is.

> Is it hard, for any who really want to know, to look inside the file
> and decide for themselves?

It is hard for anyone but you to know what is your POV on this issue.

Re: [PATCH] color-hsv-to-rgb

[Eli Zaretskii]

> From: Dov Grobgeld <dov.grobgeld@gmail.com>
> Date: Mon, 16 Oct 2023 10:28:44 +0300
> 
> I would like to do my first(!) contribution to emacs with the attached patch. Is there interest, or is it too
> trivial? Do I need to do a copyright assignment? Is there a guide for dummies on how to do that?

Thanks, see Stefan's response for some of the answers.

> diff --git a/lisp/color.el b/lisp/color.el
> index f68cf5e6b17..37f11f23c7c 100644
> --- a/lisp/color.el
> +++ b/lisp/color.el

This needs a ChangeLog-style commit log message (see CONTRIBUTE for
our expectations and "git log" for examples).

> +(defun color-hsv-to-rgb (H S V)
> +  "Convert hue, saturation and value to their RGB representation.
> +H, S, and V should each be numbers between 0.0 and 1.0, inclusive.

Our conventions is to mention the argument names on the first line

> +Return a list (RED GREEN BLUE), where each element is between 0.0 and 1.0,
> +inclusive."
> +  (let* ((I (floor (* 6 H)))
> +         (F (- (* H 6) I))
> +         (P (* V (- 1.0 S)))
> +         (Q (* V (- 1.0 (* F S))))
> +         (T (* V (- 1.0 (* (- 1.0 F) S))))
> +         (I (mod I 6)))
> +    (cond
> +     ((= I 0) (list V T P))
> +     ((= I 1) (list Q V P))
> +     ((= I 2) (list P V T))
> +     ((= I 3) (list P Q V))
> +     ((= I 4) (list T P V))
> +     ((= I 5) (list V P Q))
> +     (t (list 0 0 0)))))
> +

Would you mind adding tests for this function to
test/lisp/color-tests.el?

Thanks.

Re: [PATCH] color-hsv-to-rgb

[Dov Grobgeld]

[-- Attachment #1: Type: text/plain, Size: 2189 bytes --]

Thanks Eli and Stefan,

I'm working on the copyright assignment.

Regarding Stefan's question about the use case for hsv-to-rgb. I was
playing around with https://github.com/jdtsmith and I got the idea that it
would be nice to have the color change its hue as a function of the
indentation level. And I also wanted the saturation to change for the
current indentation level. Since I'm using a bright background, this gets
easy in HSV space. So that's the back story.

Regards and with hope for better times,

On Mon, Oct 16, 2023 at 3:05 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Dov Grobgeld <dov.grobgeld@gmail.com>
> > Date: Mon, 16 Oct 2023 10:28:44 +0300
> >
> > I would like to do my first(!) contribution to emacs with the attached
> patch. Is there interest, or is it too
> > trivial? Do I need to do a copyright assignment? Is there a guide for
> dummies on how to do that?
>
> Thanks, see Stefan's response for some of the answers.
>
> > diff --git a/lisp/color.el b/lisp/color.el
> > index f68cf5e6b17..37f11f23c7c 100644
> > --- a/lisp/color.el
> > +++ b/lisp/color.el
>
> This needs a ChangeLog-style commit log message (see CONTRIBUTE for
> our expectations and "git log" for examples).
>
> > +(defun color-hsv-to-rgb (H S V)
> > +  "Convert hue, saturation and value to their RGB representation.
> > +H, S, and V should each be numbers between 0.0 and 1.0, inclusive.
>
> Our conventions is to mention the argument names on the first line
>
> > +Return a list (RED GREEN BLUE), where each element is between 0.0 and
> 1.0,
> > +inclusive."
> > +  (let* ((I (floor (* 6 H)))
> > +         (F (- (* H 6) I))
> > +         (P (* V (- 1.0 S)))
> > +         (Q (* V (- 1.0 (* F S))))
> > +         (T (* V (- 1.0 (* (- 1.0 F) S))))
> > +         (I (mod I 6)))
> > +    (cond
> > +     ((= I 0) (list V T P))
> > +     ((= I 1) (list Q V P))
> > +     ((= I 2) (list P V T))
> > +     ((= I 3) (list P Q V))
> > +     ((= I 4) (list T P V))
> > +     ((= I 5) (list V P Q))
> > +     (t (list 0 0 0)))))
> > +
>
> Would you mind adding tests for this function to
> test/lisp/color-tests.el?
>
> Thanks.
>

[-- Attachment #2: Type: text/html, Size: 3386 bytes --]

RE: [External] : Re: [PATCH] color-hsv-to-rgb

[Drew Adams]

> It is hard for anyone but you to know what
> is your POV on this issue.

I don't have a POV on what else in `hexrgb.el'
might be missing from `color.el' and might want
to be added now by Emacs Dev.  And that's what
I said, at the outset of my reply to Stefan's
question:

  "Sorry, but I don't know"

All of hexrgb.el was considered when `color.el'
was being created, including `hexrgb-hsv-to-rgb'.
Whatever was wanted was used, and that didn't
include `hexrgb-hsv-to-rgb'.

I mentioned that function in this thread only
because of the thread Subject.  Would you have
preferred that I not mention it?  I figured
its code might help in some way, as well as
the fact that I (too) think such a function is
useful.  The latter was the main reason I wrote
to the thread: to say +1 to Jim's +1 to Dov's
request.  That's the main msg: it's useful.

There's no need for me to guess what else you
might be interested in.  If you're interested,
just take a look at the available functions.
It's not like there are zillions of them, and
they're all handily listed in one place, with
clear names.

As I said, I haven't looked at this code since
2018, and likewise `color.el'.  If someone is
interested in investigating the differences
they are as capable as I am for that.

And again, I have no POV about "main things 
besides `color-hsv-to-rgb'".
___

A quick look now at `color.el' shows it's
mostly about color conversion, not color
modification, HSL being the sole exception,
and only for S and L. `hexrgb.el' has eight
functions for incrementing/decrementing:

  +/- R, G, B individually and together
  +/- H, S, V individually
  +/- hex digits together

Why `color.el' has separate functions (8 of
them!) for the two directions to increment
only S and L is a mystery.  With `hexrgb.el',
just use a negative increment to decrement.

`hexrgb.el' also has functions to modify a
color by complementing only one component: red/green/blue/hue/saturation/value.  E.g.,
complementing hue doesn't change saturation
or value.

  (hexrgb-hue-complement "LightBlue")
    ; => "#E6E5BBBAADAC"

HTH.