bug#25890: `color-values` gives wrong value

bug#25890: `color-values` gives wrong value

[Rasmus]

Hi Eli,

Eli Zaretskii <eliz@gnu.org> writes:

>> I try to get the color value of my background, "light yellow", with
>> color.el.  However, `color-values` from faces.el returns the wrong value.
>> 
>>   (color-values "#ffffff")      => (65280 65280 65280) ; The max values.
>
> Are you sure?
>
> What does the below produce?
>
>    (color-values "#fffffffff")

For that I get the right value:

    (color-values "#fffffffff") => (65520 65520 65520)
    (color-values "#ffffff")    => (65280 65280 65280)

>>   (color-values "#ffffe0")      => (65280 65280 57344) ; Light yellow hex.
>>   (color-values "light yellow") => (65535 65535 57568) ; The potential bug.
>
> On my system, I get these results:
>
>   (color-values "#ffffff")      => (65535 65535 65535)
>   (color-values "#ffffe0")      => (65535 65535 57568)
>   (color-values "light yellow") => (65535 65535 57568)

Which seems more reasonable.

>> That is the color value of "light yellow" exceeds the maximum value as
>> defined by the color values of white (cf. `color-rgb-to-hex').  This means
>> that the color.el functions won't work.
>
> I think this means your color resolution is greater than 24 bits.

Xorg tells me it's using 24 bits.

    $> xwininfo -root | grep Depth
    Depth: 24

> Or maybe I don't understand how the above affects your code.

I want to manipulate colors such as "light yellow" with color.el.  E.g.

    (set-frame-parameter (selected-frame) 'background-color
                         (color-darken-name "light yellow" 5))

This produces an "Undefined color" error because

    (color-darken-name "light yellow" 5) => "#100100c6".

This in turn points to `color-name-to-rgb', which uses (color-values
"#ffffff") as the denominator.  Because "#ffffff produces an RGB triplet
with smaller numbers than "light yellow", I get an RGB triple with
elements exceeding 1, which it shouldn't.

So an easy fix might be to use "#fffffffff" in `color-name-to-rgb', but
then I'd still not have equal color values for the white with and without
alpha...

    (equal (color-values "#fffffffff") (color-values "#ffffff")) => nil

Thus, I suggested that the error was in the C code in my initial report.
I hope this explains the problem I'm seeing better.

Thanks,
Rasmus

-- 
Even a three-legged dog has three good legs to lose

bug#25890: `color-values` gives wrong value

[Rasmus]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> >   (color-values "#ffffff") => (65280 65280 65280) ; The max values.
>>
>> Are you sure?
>
> FWIW, I see the same here; and we already have a bug report like this
> one:
>
>   https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24273

Oh thanks.  (I didn't search as gmane search is still down).

I guess this bug can be closed.

Rasmus

-- 
⠠⠵

bug#25890: `color-values` gives wrong value

[Eli Zaretskii]

> From: Rasmus <rasmus@gmx.us>
> Cc: eliz@gnu.org, 25890@debbugs.gnu.org
> Date: Tue, 28 Feb 2017 10:44:22 +0100
> 
> Michael Heerdegen <michael_heerdegen@web.de> writes:
> 
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> >> >   (color-values "#ffffff") => (65280 65280 65280) ; The max values.
> >>
> >> Are you sure?
> >
> > FWIW, I see the same here; and we already have a bug report like this
> > one:
> >
> >   https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24273
> 
> Oh thanks.  (I didn't search as gmane search is still down).
> 
> I guess this bug can be closed.

I'd rather we fixed it.

I think there's a bug in color.el: it assumes that the #RGB notation
uses 24 bits, i.e. each component maxes out at 255.  But Emacs does
its color calculations based on 16-bit components, which max out at
65535, which is why #ffffff does NOT produce (65535 65535 65535), the
white color.  (We decided long ago to use 16-bit components because X
APIs such as XParseColor on some systems, including yours, work with
such components.)  IOW, #ffffff is parsed as #ff00ff00ff00, and that
is not "white".

So I think the patch below is what is needed to fix this problem.  Can
you try it?  (There's one user of the functions the patch changes,
shr-color.el, which will need to be adapted to the change, if we
decide to install it.)


--- lisp/color.el~0	2017-01-02 06:25:09.000000000 +0200
+++ lisp/color.el	2017-02-28 17:35:43.570586100 +0200
@@ -52,14 +52,14 @@
 If FRAME cannot display COLOR, return nil."
   ;; `colors-values' maximum value is either 65535 or 65280 depending on the
   ;; display system.  So we use a white conversion to get the max value.
-  (let ((valmax (float (car (color-values "#ffffff")))))
+  (let ((valmax (float (car (color-values "#ffffffffffff")))))
     (mapcar (lambda (x) (/ x valmax)) (color-values color frame))))
 
 (defun color-rgb-to-hex  (red green blue)
   "Return hexadecimal notation for the color RED GREEN BLUE.
 RED, GREEN, and BLUE should be numbers between 0.0 and 1.0, inclusive."
-  (format "#%02x%02x%02x"
-          (* red 255) (* green 255) (* blue 255)))
+  (format "#%04x%04x%04x"
+          (* red 65535) (* green 65535) (* blue 65535)))
 
 (defun color-complement (color-name)
   "Return the color that is the complement of COLOR-NAME.

bug#25890: `color-values` gives wrong value

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> So I think the patch below is what is needed to fix this problem.  Can
> you try it?  (There's one user of the functions the patch changes,
> shr-color.el, which will need to be adapted to the change, if we
> decide to install it.)
>
>
> --- lisp/color.el~0	2017-01-02 06:25:09.000000000 +0200
> +++ lisp/color.el	2017-02-28 17:35:43.570586100 +0200
> @@ -52,14 +52,14 @@
>  If FRAME cannot display COLOR, return nil."
>    ;; `colors-values' maximum value is either 65535 or 65280 depending on the
>    ;; display system.  So we use a white conversion to get the max value.
> -  (let ((valmax (float (car (color-values "#ffffff")))))
> +  (let ((valmax (float (car (color-values "#ffffffffffff")))))
>      (mapcar (lambda (x) (/ x valmax)) (color-values color frame))))
>  
>  (defun color-rgb-to-hex  (red green blue)
>    "Return hexadecimal notation for the color RED GREEN BLUE.
>  RED, GREEN, and BLUE should be numbers between 0.0 and 1.0, inclusive."
> -  (format "#%02x%02x%02x"
> -          (* red 255) (* green 255) (* blue 255)))
> +  (format "#%04x%04x%04x"
> +          (* red 65535) (* green 65535) (* blue 65535)))
>  
>  (defun color-complement (color-name)
>    "Return the color that is the complement of COLOR-NAME.

With that patch, for me in the recipe of bug #24273, it makes a
difference; now

(color-name-to-rgb "#ffffff")
==>
(0.9961089494163424 0.9961089494163424 0.9961089494163424)

and

(color-name-to-rgb "white")
==>
(1.0 1.0 1.0)


For the recipe of this bug report, the values are the same as without
the patch.

I guess this is what you expected.  But is it ok that "#ffffff" is not
equivalent to "white"?  If it is, seems there is no inconsistency left.


Thanks so far,

Michael.

bug#25890: `color-values` gives wrong value

[Drew Adams]

> >  (defun color-rgb-to-hex  (red green blue)
> >    "Return hexadecimal notation for the color RED GREEN BLUE.
> >  RED, GREEN, and BLUE should be numbers between 0.0 and 1.0, inclusive."
> > -  (format "#%02x%02x%02x"
> > -          (* red 255) (* green 255) (* blue 255)))
> > +  (format "#%04x%04x%04x"
> > +          (* red 65535) (* green 65535) (* blue 65535)))

The function should accept an optional arg NB-DIGITS, which
specifies the number of hex digits for each of R, G, B.  And
yes, it should default to 4 digits: #RRRRGGGGBBBB.

(That's what the original function in hexrgb.el does, from
which color.el was supposedly derived.)

bug#25890: `color-values` gives wrong value

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: Rasmus <rasmus@gmx.us>,  25890@debbugs.gnu.org
> Date: Wed, 01 Mar 2017 00:42:02 +0100
> 
> With that patch, for me in the recipe of bug #24273, it makes a
> difference; now
> 
> (color-name-to-rgb "#ffffff")
> ==>
> (0.9961089494163424 0.9961089494163424 0.9961089494163424)
> 
> and
> 
> (color-name-to-rgb "white")
> ==>
> (1.0 1.0 1.0)

That's what should happen, yes.

> For the recipe of this bug report, the values are the same as without
> the patch.
> 
> I guess this is what you expected.

Yes, expected.  color-values were not affected by the change, it's
just that color.el is now consistent with what color-values returns.

> But is it ok that "#ffffff" is not equivalent to "white"?

Yes.  There's no other way, really, on systems where "white" has 3
16-bit components with all bits set, when color-values is called.

bug#25890: `color-values` gives wrong value

[Rasmus]

Hi,

Eli Zaretskii <eliz@gnu.org> writes:

> I think there's a bug in color.el: it assumes that the #RGB notation
> uses 24 bits, i.e. each component maxes out at 255.  But Emacs does
> its color calculations based on 16-bit components, which max out at
> 65535, which is why #ffffff does NOT produce (65535 65535 65535), the
> white color.  (We decided long ago to use 16-bit components because X
> APIs such as XParseColor on some systems, including yours, work with
> such components.)  IOW, #ffffff is parsed as #ff00ff00ff00, and that
> is not "white".
>
> So I think the patch below is what is needed to fix this problem.  Can
> you try it?  (There's one user of the functions the patch changes,
> shr-color.el, which will need to be adapted to the change, if we
> decide to install it.)

The test I used before now works for me:

    (set-frame-parameter (selected-frame) 'background-color
                         (color-darken-name "light yellow" 5))

Other noteworthy effects of the change were already addressed by Michael.

Thanks,
Rasmus

-- 
Hvor meget poesi tror De kommer ud af et glas isvand?

bug#25890: `color-values` gives wrong value

[Eli Zaretskii]

> From: Rasmus <rasmus@gmx.us>
> Date: Wed, 01 Mar 2017 10:55:22 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I think there's a bug in color.el: it assumes that the #RGB notation
> > uses 24 bits, i.e. each component maxes out at 255.  But Emacs does
> > its color calculations based on 16-bit components, which max out at
> > 65535, which is why #ffffff does NOT produce (65535 65535 65535), the
> > white color.  (We decided long ago to use 16-bit components because X
> > APIs such as XParseColor on some systems, including yours, work with
> > such components.)  IOW, #ffffff is parsed as #ff00ff00ff00, and that
> > is not "white".
> >
> > So I think the patch below is what is needed to fix this problem.  Can
> > you try it?  (There's one user of the functions the patch changes,
> > shr-color.el, which will need to be adapted to the change, if we
> > decide to install it.)
> 
> The test I used before now works for me:
> 
>     (set-frame-parameter (selected-frame) 'background-color
>                          (color-darken-name "light yellow" 5))
> 
> Other noteworthy effects of the change were already addressed by Michael.

Thanks, I installed my change with minor variations, and I'm marking
this bug done.

bug#25890: `color-values` gives wrong value

[Eli Zaretskii]

> Date: Tue, 28 Feb 2017 15:54:46 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 25890@debbugs.gnu.org, Rasmus <rasmus@gmx.us>
> 
> > >  (defun color-rgb-to-hex  (red green blue)
> > >    "Return hexadecimal notation for the color RED GREEN BLUE.
> > >  RED, GREEN, and BLUE should be numbers between 0.0 and 1.0, inclusive."
> > > -  (format "#%02x%02x%02x"
> > > -          (* red 255) (* green 255) (* blue 255)))
> > > +  (format "#%04x%04x%04x"
> > > +          (* red 65535) (* green 65535) (* blue 65535)))
> 
> The function should accept an optional arg NB-DIGITS, which
> specifies the number of hex digits for each of R, G, B.  And
> yes, it should default to 4 digits: #RRRRGGGGBBBB.
> 
> (That's what the original function in hexrgb.el does, from
> which color.el was supposedly derived.)

The code in hexrgb.el produces strange results in this regard (e.g.,
it produces "#FFFFFFFFE0E0" instead of "#FFFFFFFFE000" for the color
mentioned by the OP).  I believe that's because it interprets the
conversion between 2 and 4 hex digits incorrectly: the 2 hex digits
are the _most_ significant bits of the 4-digit version, not the LSB.

But I did add such an optional argument to color-rgb-to-hex, with the
difference that its value can only be either 4 or 2, as I see no
reason for anyone to want a 12-bit per component color notations.

Thanks.

bug#25890: `color-values` gives wrong value

[Drew Adams]

> > > >  (defun color-rgb-to-hex  (red green blue)
> >
> > The function should accept an optional arg NB-DIGITS, which
> > specifies the number of hex digits for each of R, G, B.  And
> > yes, it should default to 4 digits: #RRRRGGGGBBBB.
> >
> > (That's what the original function in hexrgb.el does, from
> > which color.el was supposedly derived.)
> 
> The code in hexrgb.el produces strange results in this regard (e.g.,
> it produces "#FFFFFFFFE0E0" instead of "#FFFFFFFFE000" for the color
> mentioned by the OP).

Not clear what you are saying.  What color mentioned by the OP?
Do you mean "light yellow"?  What sexp using hexrgb.el did you try?  

If I do (hexrgb-color-name-to-hex "light yellow") I do get
"#FFFFFFFFE0E0".  That comes from `x-color-values' returning
(65535 65535 57568) and `hexrgb-int-to-hex' converting 57568
to "E0E0".  That's from (format "%04X" 57568).  Hex conversion
of decimal 57568 _should_ be E0E0, AFAIK.  Where is the bug?

Or are you saying that the bug is from `x-color-values'
(`color-values') and not from hexrgb.el?  Is 57568 the
wrong blue color value for "light yellow"?

> I believe that's because it interprets the
> conversion between 2 and 4 hex digits incorrectly: the 2 hex digits
> are the _most_ significant bits of the 4-digit version, not the LSB.

See above.  I'm missing what you are trying to say, I guess.
Are you saying that color value 57568 should not be expressed
in hex as E0E0?

> But I did add such an optional argument to color-rgb-to-hex,

Good.

> with the difference that its value can only be either 4 or 2,
> as I see no reason for anyone to want a 12-bit per component
> color notations.

(I see no reason for the function not to be general.  Sure,
for current applications of such a function to colors there
is no real need for such generality.  But why prevent it?)

Anyway, thanks for adding the optional arg.

I'm interested in your reply to my questions above,
especially in the case that might have located a bug in
hexrgb.el.  It's not clear to me, so far.  Thx.

bug#25890: `color-values` gives wrong value

[Eli Zaretskii]

> Date: Fri, 3 Mar 2017 07:49:33 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: michael_heerdegen@web.de, 25890@debbugs.gnu.org, rasmus@gmx.us
> 
> > The code in hexrgb.el produces strange results in this regard (e.g.,
> > it produces "#FFFFFFFFE0E0" instead of "#FFFFFFFFE000" for the color
> > mentioned by the OP).
> 
> Not clear what you are saying.  What color mentioned by the OP?
> Do you mean "light yellow"?  What sexp using hexrgb.el did you try?  
> 
> If I do (hexrgb-color-name-to-hex "light yellow") I do get
> "#FFFFFFFFE0E0".  That comes from `x-color-values' returning
> (65535 65535 57568) and `hexrgb-int-to-hex' converting 57568
> to "E0E0".  That's from (format "%04X" 57568).  Hex conversion
> of decimal 57568 _should_ be E0E0, AFAIK.  Where is the bug?

The bug is in hexrgb-int-to-hex: it incorrectly assumes that it should
produce the LSB part of the number, while it actually should produce
the MSB part.

bug#25890: `color-values` gives wrong value

[Drew Adams]

> > > The code in hexrgb.el produces strange results in this regard (e.g.,
> > > it produces "#FFFFFFFFE0E0" instead of "#FFFFFFFFE000" for the color
> > > mentioned by the OP).
> >
> > Not clear what you are saying.  What color mentioned by the OP?
> > Do you mean "light yellow"?  What sexp using hexrgb.el did you try?
> >
> > If I do (hexrgb-color-name-to-hex "light yellow") I do get
> > "#FFFFFFFFE0E0".  That comes from `x-color-values' returning
> > (65535 65535 57568) and `hexrgb-int-to-hex' converting 57568
> > to "E0E0".  That's from (format "%04X" 57568).  Hex conversion
> > of decimal 57568 _should_ be E0E0, AFAIK.  Where is the bug?
> 
> The bug is in hexrgb-int-to-hex: it incorrectly assumes that it should
> produce the LSB part of the number, while it actually should produce
> the MSB part.

You seem to be just repeating yourself, without answering my
questions.  Are you saying that hex conversion of decimal 57568
should not be E0E0?  If so, why?  Why should it be E000?

bug#25890: `color-values` gives wrong value

[Eli Zaretskii]

> Date: Fri, 3 Mar 2017 11:20:44 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: michael_heerdegen@web.de, 25890@debbugs.gnu.org, rasmus@gmx.us
> 
> Are you saying that hex conversion of decimal 57568 should not be
> E0E0?

No.

bug#25890: `color-values` gives wrong value

[Drew Adams]

> > Are you saying that hex conversion of decimal 57568 should not be
> > E0E0?
> 
> No.

Not very helpful.  It's not clear what you think is incorrect
in hexrgb.el, in that case.

You said:

> The bug is in hexrgb-int-to-hex: it incorrectly assumes that it should
> produce the LSB part of the number, while it actually should produce
> the MSB part.

`hexrgb-int-to-hex' just converts integer RGB to hex RGB.
And it converts integer 57568 to E0E0, which you now agree
is correct.

Perhaps you can elaborate a bit on what you think the bug is?

bug#25890: `color-values` gives wrong value

[Eli Zaretskii]

> Date: Fri, 3 Mar 2017 13:18:12 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: michael_heerdegen@web.de, 25890@debbugs.gnu.org, rasmus@gmx.us
> 
> You said:
> 
> > The bug is in hexrgb-int-to-hex: it incorrectly assumes that it should
> > produce the LSB part of the number, while it actually should produce
> > the MSB part.
> 
> `hexrgb-int-to-hex' just converts integer RGB to hex RGB.
> And it converts integer 57568 to E0E0, which you now agree
> is correct.
> 
> Perhaps you can elaborate a bit on what you think the bug is?

I already did, but you didn't want to hear, claiming that I repeat
myself.

Here it is again: the problem is in hexrgb-int-to-hex.  It does this:

  (substring (format (concat "%0" (int-to-string nb-digits) "X") int) (- nb-digits))

which assumes that the digits to be produced, if n in the %0nX format
is too small and doesn't allow to produce all of them, are the
least-significant digits of the number.  It should instead produce the
most-significant digits.  IOW, when #FFFFFF is converted to 16-bit per
color component, it should yield #FF00FF00FF00, not #FFFFFFFFFFFF.

bug#25890: `color-values` gives wrong value

[Simen Heggestøyl]

On Fri, Mar 3, 2017 at 3:08 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> Thanks, I installed my change with minor variations, and I'm marking
> this bug done.

Thanks, Eli. The changes makes sense to me, but I've still got one
problem. I'm working with web colors, where color codes are specified
with either one or two digits per component (meaning that both "#fff"
and "#ffffff" specify the brightest possible value, named
"white"). But:

(apply #'color-rgb-to-hex `(,@(color-name-to-rgb "#ffffff") 2))
     => "#fefefe"

(apply #'color-rgb-to-hex `(,@(color-name-to-rgb "#fff") 2))
     => "#efefef"

Where I would like both to give back "#ffffff". Does this mean that I
have to ensure that input to `color-name-to-rgb' uses 4 digits per
component? Would it make sense for it to get a new optional parameter
`DIGITS-PER-COMPONENT', like `color-rgb-to-hex' did?

-- Simen

bug#25890: `color-values` gives wrong value

[Eli Zaretskii]

> Date: Sat, 04 Mar 2017 15:24:00 +0100
> From: Simen Heggestøyl <simenheg@gmail.com>
> Cc: 25890-done@debbugs.gnu.org
> 
> Thanks, Eli. The changes makes sense to me, but I've still got one
> problem. I'm working with web colors, where color codes are specified
> with either one or two digits per component (meaning that both "#fff"
> and "#ffffff" specify the brightest possible value, named
> "white"). But:
> 
> (apply #'color-rgb-to-hex `(,@(color-name-to-rgb "#ffffff") 2))
>      => "#fefefe"
> 
> (apply #'color-rgb-to-hex `(,@(color-name-to-rgb "#fff") 2))
>      => "#efefef"
> 
> Where I would like both to give back "#ffffff". Does this mean that I
> have to ensure that input to `color-name-to-rgb' uses 4 digits per
> component?

Yes, IMO.  Emacs does all its calculations with color components
assuming 16 bits per component.  Applications that need to use fewer
digits and have the missing digits as something other than zero, need
to do that in application code.

> Would it make sense for it to get a new optional parameter
> `DIGITS-PER-COMPONENT', like `color-rgb-to-hex' did?

Not IMO, because only the application knows what it means by #ffffff,
color.el has no way of knowing that.  So it would be best to leave
this for the application to do, by converting the #RGB spec to the
canonical 16-bits-per-component form.

bug#25890: `color-values` gives wrong value

[Drew Adams]

> the problem is in hexrgb-int-to-hex.  It does this:
> 
>   (substring (format (concat "%0" (int-to-string nb-digits) "X") int)
>              (- nb-digits))
> 
> which assumes that the digits to be produced, if n in the %0nX format
> is too small and doesn't allow to produce all of them, are the
> least-significant digits of the number.

Correct.  Which is just what its doc string says:

 If INT is too large to be represented with NB-DIGITS, then the result
 is truncated from the left.  So, for example, INT=256 and NB-DIGITS=2
 returns \"00\", since the hex equivalent of 256 decimal is 100, which
 is more than 2 digits.

That doesn't claim that the result faithfully represents the decimal
input value.  It doesn't.  But neither would truncating from the
right represent it correctly.  The doc string just describes the
behavior, letting you know what the (inaccurate) return value is
in such a case.

Is there an advantage in truncating from the right?  If so,
what?  Can you show how you would use such "corrected" behavior
advantageously?  To my mind, neither kind of truncation gives
you anything useful, if what you need is to represent the same
number faithfully.

> It should instead produce the most-significant digits.

Why?  How would that be better?  If the decimal number is too
big for the allotted number of hex digits then it is just too big.

> IOW, when #FFFFFF is converted to 16-bit per color component, it
> should yield #FF00FF00FF00, not #FFFFFFFFFFFF.

Where do you see `hexrgb-int-to-hex' being used as (I guess) you
suppose?  Where do you see #FFFFFF "converted" to #FFFFFFFFFFFF
(let alone to #FF00FF00FF00).

Sorry, but I just do not understand what you're trying to say.

Are you perhaps thinking of using `hexrgb-int-to-hex' together
with `hexrgb-hex-to-int', to "convert" from 6 hex digits to 12?
In that case, the "conversion" would be from "FFFFFF" to
"000000FFFFFF", which is correct, I think:

(hexrgb-int-to-hex (hexrgb-hex-to-int "FFFFFF") 12) => "000000FFFFFF"

Or from 2 to 4 hex digits (since the function is used on each
color component separately):

(hexrgb-int-to-hex (hexrgb-hex-to-int "FF") 4) => "00FF"

Or for your "E0" example:

(hexrgb-int-to-hex (hexrgb-hex-to-int "E0") 4) => "00E0"

I don't see any "conversion" from "E0" to "E0E0" (or to "E000",
which is apparently what you suggest is called for).

Can you please relate your message, whatever it might be, back
to your initial statement that hexrgb 'produces "#FFFFFFFFE0E0"
instead of "#FFFFFFFFE000" for the color mentioned by the OP'.

AFAIK, the color mentioned by the OP was "light yellow".  Do
we agree so far?  (I asked this once, but got no reply.)

(And I asked what sexp using the hexrgb code you tried, but I
got no answer there either.)

I said that if you use (hexrgb-color-name-to-hex "light yellow")
which is what I'd hope you would use, you do get "#FFFFFFFFE0E0".
And I said that that is, AFAIK, correct, not incorrect.

You say (I think) that the correct hex value for "light yellow"
is, or should be, "#FFFFFFFFE000".  But you haven't said why you
think that.

The color values for "light yellow" are: (65535 65535 57568).
I asked if you thought those values are correct, but I got no
reply.

Let's assume they are correct (they are not from hexrgb code,
anyway).  If so, the question is how each of those color values
should be represented using 4 hex digits.  `hexrgb-int-to-hex'
returns "FFFF" for each of the first two and "E0E0" for the third.

I asked if you agreed that that is correct, and you said yes.
To me, that means that it is correct for the 12-digit hex code
for "light yellow" to be "#FFFFFFFFE0E0", which you apparently
do not agree with.

I get the impression that you are perhaps thinking that it is
about "converting" FFFFE0 (not "light yellow") to FFFFFFFFE0E0
or to FFFFFFFFE000.  If so, why do you think that, and where
do you see the hexrgb code doing any such "conversion"?

Sorry, but I just do not get your message.  Could you perhaps
show some code that points out the bug you think you have found?
Could you show (with code) how using `hexrgb-int-to-hex' for
"light yellow" or whatever gives the wrong hex representation?

Saying that `hexrgb-int-to-hex' truncates from the left (i.e.,
drops the MSB) just repeats what its doc string says.  It's not
a bug but the declared behavior.

If you see a _use_ of `hexrgb-int-to-hex' in hexrgb.el that
leads to incorrect results because of that (intended) behavior,
please point to it.

I'd like to correct a bug in hexrgb, but so far I haven't been
able to see what it is.

You apparently think that `hexrgb-int-to-hex' should truncate
from the right, i.e., drop the LSB, not the MSB, when the number
is too large to be represented by the given number of hex digits.

I don't see the advantage of that.  If the number is too large
then it is too large.  For an integer value of 256 you would, I
guess, prefer to see the hex representation "10" returned instead
of "00".  I don't see how that would help anything.

I think the question is how code makes _use_ of `hexrgb-int-to-hex'.
If the number to convert is too big for the number of hex digits
allowed then there is no way to represent the decimal number with
those few hex digits.  I don't see an advantage to truncating from
the right instead of the left.

Can you give an actual example, where you would use a version of
`hexrgb-int-to-hex' that is corrected according to you (presumably
truncating at the right), where you can usefully make use of the
return value?  IOW, supposing a corrected `hexrgb-int-to-hex',
how would you use it in a sexp to take advantage of the correction?

AFAICS, if the decimal value cannot be represented with so few hex
digits there is no sense in trying to use the result as if it
represented the same number.  But perhaps I'm missing something.

bug#25890: `color-values` gives wrong value

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> (apply #'color-rgb-to-hex `(,@(color-name-to-rgb "#ffffff") 2))
>>      => "#fefefe"
>> 
>> (apply #'color-rgb-to-hex `(,@(color-name-to-rgb "#fff") 2))
>>      => "#efefef"
>> 
>> Where I would like both to give back "#ffffff". Does this mean that I
>> have to ensure that input to `color-name-to-rgb' uses 4 digits per
>> component?
>
> Yes, IMO.  Emacs does all its calculations with color components
> assuming 16 bits per component.  Applications that need to use fewer
> digits and have the missing digits as something other than zero, need
> to do that in application code.

I haven't followed this discussion closely, but you've changed the way
color-rgb-to-rgb works in an incompatible way?  I don't think that makes
much sense -- it's a function we must assume is used by third party code
and that's supposed to return certain values on certain inputs.

If you want to have a function that does something else, I think it
would make more sense to introduce new functions that does this
"something new" (i.e., 16 bit component parsing instead of HTML-like
color parsing, which is what those functions are supposed to do), and
revert the old functions how they used to work.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#25890: `color-values` gives wrong value

[Eli Zaretskii]

> Date: Sat, 4 Mar 2017 07:38:51 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: michael_heerdegen@web.de, 25890@debbugs.gnu.org, rasmus@gmx.us
> 
> > It should instead produce the most-significant digits.
> 
> Why?

Because that's how colors are interpreted on X, which is where Emacs
takes its ideas of how to process these specs.  See, for example, this
man page:

  https://linux.die.net/man/3/xparsecolor

(Emacs on X uses XParseColor internally in the implementation of
color-values.)

If you do it differently, sooner or later the results will clash with
what Emacs does, and yield subtly wrong values.

I will now bail out of this discussion.  I have nothing more to say
about this issue.  If you don't want to change hexrgb.el, that's your
prerogative.

bug#25890: `color-values` gives wrong value

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Simen Heggestøyl <simenheg@gmail.com>,
>   25890@debbugs.gnu.org,  rasmus@gmx.us
> Date: Sat, 04 Mar 2017 16:44:28 +0100
> 
> I haven't followed this discussion closely, but you've changed the way
> color-rgb-to-rgb works in an incompatible way?

There's no color-rgb-to-rgb.  There's color-rgb-to-hex and
color-name-to-rgb.  And I don't think the changes are incompatible.
Please show examples if you think otherwise.

> If you want to have a function that does something else, I think it
> would make more sense to introduce new functions that does this
> "something new" (i.e., 16 bit component parsing instead of HTML-like
> color parsing, which is what those functions are supposed to do), and
> revert the old functions how they used to work.

The changes make the above 2 functions compatible with all the other
color calculations in Emacs.  IMO it makes no sense to have color.el
make assumptions about RGB components that are different from
everything else in Emacs, as doing so will only produce subtle bugs
such as the one which started this bug report.

bug#25890: `color-values` gives wrong value

[Drew Adams]

> > > It should instead produce the most-significant digits.
> >
> > Why?
> 
> Because that's how colors are interpreted on X,

I think you misunderstand `hexrgb-int-to-hex'.  It does
no color interpretation at all.

It simply converts a decimal integer to a string of hex
digits of a given length.  If the prescribed length is
too short for the number then the returned string clearly
cannot represent the number accurately - as is documented.

The fact that too few digits cannot represent the number
is irrespective of whether those too few digits are MSB
or LSB.

It's up to code calling the function to DTRT wrt the
number of digits it passes, just as it is up to the
caller of `hexrgb-hex-to-int' to pass a hex string that
does not represent an integer larger than what Emacs
can handle.

Here, for instance, is how `hexrgb-increment-hex' checks
inputs to `hexrgb-int-to-hex' before it calls it:

 (<= (length (format (concat \"%X\") INT)) NB-DIGITS)

The doc makes all of this pretty clear, I think.

> which is where Emacs takes its ideas of how to
> process these specs.

Emacs interprets color specs.  hexrgb does not.

> See, for example, this man page:
> https://linux.die.net/man/3/xparsecolor
> (Emacs on X uses XParseColor internally in the
> implementation of color-values.)

I see nothing on that page that conflicts with the hexrgb code.
In particular, this quote describes the hexrgb behavior too:

  "the string ''#3a7'' is the same as ''#3000a0007000''

Same color represented by both patterns.

You use `hexrgb-int-to-hex' on each color component separately,
and the maximum integer input is 65535 - as documented.

(hexrgb-int-to-hex (hexrgb-hex-to-int "7000") 4 ) ==> "7000"
(hexrgb-int-to-hex (hexrgb-hex-to-int "7")    1 ) ==> "7"

It is Emacs, not hexrgb, that interprets such a hex value
as a color component.  And Emacs (correctly) interprets
the component "7" in "#3a7" the same as it interprets the
component "7000" in "#3000a0007000" - same color.

> If you do it differently, sooner or later the results will
> clash with what Emacs does, and yield subtly wrong values.

hexrgb does not "do it" at all.  hexrgb does not conflict
with anything shown on the page you cited.

> I will now bail out of this discussion.  I have nothing more
> to say about this issue.  If you don't want to change
> hexrgb.el, that's your prerogative.

I've been clear that I would LOVE to fix a bug, if you can
please point to one.  Show some code that uses hexrgb and
produces an incorrect result, for example.

bug#25890: `color-values` gives wrong value

[mail]

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

Hello,

I'd like to ask a question regarding to the commit 7b00e956.
I quickly scanned the discussion here, but couldn't find any good reason for changing the behavior of color-rgb-to-hex function without keeping backward compatibility;
why the default value for digits-per-component is 4, not 2?
This change actually had impact on several packages including mode-icons, and IMO keeping the original behavior is a better option.

Thanks,
kissge

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

bug#25890: `color-values` gives wrong value

[Eli Zaretskii]

> From: "mail@yo.eki.do" <mail@yo.eki.do>
> Date: Fri, 10 Mar 2017 05:23:35 +0000
> 
> I'd like to ask a question regarding to the commit 7b00e956.
> I quickly scanned the discussion here, but couldn't find any good reason for changing the behavior of
> color-rgb-to-hex function without keeping backward compatibility;
> why the default value for digits-per-component is 4, not 2?

The default was changed to 4 to match how Emacs does color
calculations internally.  Using 2 will produce subtle bugs, like that
one which prompted that change.

> This change actually had impact on several packages including mode-icons, and IMO keeping the original
> behavior is a better option.

I'm sorry to hear that, but you could still use the optional argument
to get back the original behavior, right?

bug#25890: `color-values` gives wrong value

[mail]

2017-03-10 16:28 GMT+09:00 Eli Zaretskii <eliz@gnu.org>:
>
> > From: "mail@yo.eki.do" <mail@yo.eki.do>
> > Date: Fri, 10 Mar 2017 05:23:35 +0000
> >
> > I'd like to ask a question regarding to the commit 7b00e956.
> > I quickly scanned the discussion here, but couldn't find any good reason for changing the behavior of
> > color-rgb-to-hex function without keeping backward compatibility;
> > why the default value for digits-per-component is 4, not 2?
>
> The default was changed to 4 to match how Emacs does color
> calculations internally.  Using 2 will produce subtle bugs, like that
> one which prompted that change.

Thanks for clear explanation.
So that basically means this destructive change was inevitable.
I guess you are right.

> > This change actually had impact on several packages including mode-icons, and IMO keeping the original
> > behavior is a better option.
>
> I'm sorry to hear that, but you could still use the optional argument
> to get back the original behavior, right?

Hopefully. Actually I'm not qute certain how many packages will be affected.

bug#25890: `color-values` gives wrong value

[mail]

2017-03-10 17:13 GMT+09:00 mail@yo.eki.do <mail@yo.eki.do>:
>> > This change actually had impact on several packages including mode-icons, and IMO keeping the original
>> > behavior is a better option.
>>
>> I'm sorry to hear that, but you could still use the optional argument
>> to get back the original behavior, right?
>
> Hopefully. Actually I'm not qute certain how many packages will be affected.

No, let me take back my words.
This change will have too large impacts since the number of parameters
will change.
Indeed developers can use the optional argument, but that trick won't
work in the previous version of Emacs and will lead to
wrong-number-of-arguments errors,
so they will need burdensome workarounds e.g. version check.
I'd have to say that the default argument should stick to 2, the original.

bug#25890: `color-values` gives wrong value

[Eli Zaretskii]

> From: "mail@yo.eki.do" <mail@yo.eki.do>
> Date: Fri, 10 Mar 2017 08:13:15 +0000
> 
> > The default was changed to 4 to match how Emacs does color
> > calculations internally.  Using 2 will produce subtle bugs, like that
> > one which prompted that change.
> 
> Thanks for clear explanation.
> So that basically means this destructive change was inevitable.
> I guess you are right.

To my defense, they didn't look so destructive when I made them.  This
function has only one caller in the rest of the Emacs sources.

> > > This change actually had impact on several packages including mode-icons, and IMO keeping the original
> > > behavior is a better option.
> >
> > I'm sorry to hear that, but you could still use the optional argument
> > to get back the original behavior, right?
> 
> Hopefully. Actually I'm not qute certain how many packages will be affected.

Neither am I.

I'm open to alternative ideas for how to solve the original bug in a
less intrusive way.  I just don't think defaulting to 2 would be such
an alternative, because the underlying issues are very subtle, and if
we continue using 2 as the default, users of that function will never
discover that they need to use 4.  But maybe there are other ways that
leave everyone more happy.

bug#25890: `color-values` gives wrong value

[Eli Zaretskii]

> From: "mail@yo.eki.do" <mail@yo.eki.do>
> Date: Fri, 10 Mar 2017 08:33:59 +0000
> 
> This change will have too large impacts since the number of parameters
> will change.
> Indeed developers can use the optional argument, but that trick won't
> work in the previous version of Emacs and will lead to
> wrong-number-of-arguments errors,
> so they will need burdensome workarounds e.g. version check.

Packages that need to support older versions of Emacs will have to
provide a wrapper function that indeed looks at emacs-version.  That's
nothing new.

> I'd have to say that the default argument should stick to 2, the original.

That would leave the original problem in place, so I don't think it's
acceptable.

bug#25890: `color-values` gives wrong value

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> I'm open to alternative ideas for how to solve the original bug in a
> less intrusive way.  I just don't think defaulting to 2 would be such
> an alternative, because the underlying issues are very subtle, and if
> we continue using 2 as the default, users of that function will never
> discover that they need to use 4.  But maybe there are other ways that
> leave everyone more happy.

The normal way to introduce incompatible changes like this is to
introduce a new function and mark the old function as obsolete, isn't
it?  (Then remove the old function in about ten years time.  :-))

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#25890: `color-values` gives wrong value

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: "mail\@yo.eki.do" <mail@yo.eki.do>,  25890@debbugs.gnu.org
> Date: Fri, 10 Mar 2017 12:07:18 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I'm open to alternative ideas for how to solve the original bug in a
> > less intrusive way.  I just don't think defaulting to 2 would be such
> > an alternative, because the underlying issues are very subtle, and if
> > we continue using 2 as the default, users of that function will never
> > discover that they need to use 4.  But maybe there are other ways that
> > leave everyone more happy.
> 
> The normal way to introduce incompatible changes like this is to
> introduce a new function and mark the old function as obsolete, isn't
> it?  (Then remove the old function in about ten years time.  :-))

I asked about that in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25525#68,
but no one answered.  The disadvantage would be that those packages
need to change their code anyway, now or in "ten years time".  If
that's deemed a better way, I can do it.

bug#25890: `color-values` gives wrong value

[mail]

2017-03-10 18:28 GMT+09:00 Eli Zaretskii <eliz@gnu.org>:
> Packages that need to support older versions of Emacs will have to
> provide a wrapper function that indeed looks at emacs-version.  That's
> nothing new.

Yes, you are right.
The point is, if it is possible, preserving the original behavior
would produce less confusion for developers.

2017-03-10 18:28 GMT+09:00 Eli Zaretskii <eliz@gnu.org>:
>> From: "mail@yo.eki.do" <mail@yo.eki.do>
>> Date: Fri, 10 Mar 2017 08:33:59 +0000
>>
>> This change will have too large impacts since the number of parameters
>> will change.
>> Indeed developers can use the optional argument, but that trick won't
>> work in the previous version of Emacs and will lead to
>> wrong-number-of-arguments errors,
>> so they will need burdensome workarounds e.g. version check.
>
> Packages that need to support older versions of Emacs will have to
> provide a wrapper function that indeed looks at emacs-version.  That's
> nothing new.
>
>> I'd have to say that the default argument should stick to 2, the original.
>
> That would leave the original problem in place, so I don't think it's
> acceptable.

bug#25890: `color-values` gives wrong value

[mail]

2017-03-10 22:32 GMT+09:00 Eli Zaretskii <eliz@gnu.org>:
> I asked about that in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25525#68,
> but no one answered.  The disadvantage would be that those packages
> need to change their code anyway, now or in "ten years time".  If
> that's deemed a better way, I can do it.

Now I understand the situation.
So far, "to introduce a new function and mark the old function as
obsolete" sounds the best idea.