* Doc string and operation of color-distance
@ 2017-09-14 18:45 Eli Zaretskii
2017-09-14 19:12 ` Mark Oteiza
2017-09-14 19:43 ` Mark Oteiza
0 siblings, 2 replies; 12+ messages in thread
From: Eli Zaretskii @ 2017-09-14 18:45 UTC (permalink / raw
To: Mark Oteiza; +Cc: emacs-devel
Mark, the new color-distance says something strange in its doc string:
(color-distance COLOR1 COLOR2 &optional FRAME METRIC)
Return an integer distance between COLOR1 and COLOR2 on FRAME.
COLOR1 and COLOR2 may be either strings containing the color name,
or lists of the form (RED GREEN BLUE), each in the range 0 to 65535 inclusive.
If FRAME is unspecified or nil, the current frame is used.
If METRIC is unspecified or nil, a modified L*u*v* metric is used.
The last sentence should say "non-nil", I think, and it should
document that METRIC is supposed to be a function of 2 colors.
Also, this is unexpected:
(color-distance "red" "blue" nil 'lcms-cam02-ucs)
=> (error "Invalid color" "red")
I think the new lcms2 functions should support colors specified as
strings, because all the other color-related functions do.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Doc string and operation of color-distance
2017-09-14 18:45 Doc string and operation of color-distance Eli Zaretskii
@ 2017-09-14 19:12 ` Mark Oteiza
2017-09-15 13:30 ` Eli Zaretskii
2017-09-14 19:43 ` Mark Oteiza
1 sibling, 1 reply; 12+ messages in thread
From: Mark Oteiza @ 2017-09-14 19:12 UTC (permalink / raw
To: Eli Zaretskii; +Cc: emacs-devel
On 14/09/17 at 09:45pm, Eli Zaretskii wrote:
> Mark, the new color-distance says something strange in its doc string:
>
> (color-distance COLOR1 COLOR2 &optional FRAME METRIC)
>
> Return an integer distance between COLOR1 and COLOR2 on FRAME.
> COLOR1 and COLOR2 may be either strings containing the color name,
> or lists of the form (RED GREEN BLUE), each in the range 0 to 65535 inclusive.
> If FRAME is unspecified or nil, the current frame is used.
> If METRIC is unspecified or nil, a modified L*u*v* metric is used.
>
> The last sentence should say "non-nil", I think
No, the 'nil' is correct--if no metric is given as an argument,
color_distance is used which, as the comments say, is a modified L*u*v*
metric.
> and it should
> document that METRIC is supposed to be a function of 2 colors.
True, that would be helpful.
> Also, this is unexpected:
>
> (color-distance "red" "blue" nil 'lcms-cam02-ucs)
> => (error "Invalid color" "red")
>
> I think the new lcms2 functions should support colors specified as
> strings, because all the other color-related functions do.
I thought (but did not test) the body of color-distance did the lookups.
Now I see what's going on.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Doc string and operation of color-distance
2017-09-14 18:45 Doc string and operation of color-distance Eli Zaretskii
2017-09-14 19:12 ` Mark Oteiza
@ 2017-09-14 19:43 ` Mark Oteiza
2017-09-14 20:23 ` Mark Oteiza
2017-09-15 13:40 ` Eli Zaretskii
1 sibling, 2 replies; 12+ messages in thread
From: Mark Oteiza @ 2017-09-14 19:43 UTC (permalink / raw
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
> Mark, the new color-distance says something strange in its doc string:
>
> (color-distance COLOR1 COLOR2 &optional FRAME METRIC)
>
> Return an integer distance between COLOR1 and COLOR2 on FRAME.
> COLOR1 and COLOR2 may be either strings containing the color name,
> or lists of the form (RED GREEN BLUE), each in the range 0 to 65535 inclusive.
> If FRAME is unspecified or nil, the current frame is used.
> If METRIC is unspecified or nil, a modified L*u*v* metric is used.
>
> The last sentence should say "non-nil", I think, and it should
> document that METRIC is supposed to be a function of 2 colors.
>
> Also, this is unexpected:
>
> (color-distance "red" "blue" nil 'lcms-cam02-ucs)
> => (error "Invalid color" "red")
>
> I think the new lcms2 functions should support colors specified as
> strings, because all the other color-related functions do.
At the very least we should feed metrics the translated RGBs.
Beyond that, in order to use (for example) lcms2-cam02-ucs with
color-distance, one has to translate RGB to XYZ, so the metric would be
(lambda (a b)
(lcms2-cam02-ucs (color-srgb-to-xyz a) (color-srgb-to-xyz b))
diff --git a/src/xfaces.c b/src/xfaces.c
index 012de4e7af..9804c96c39 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -4093,7 +4093,9 @@ DEFUN ("color-distance", Fcolor_distance, Scolor_distance, 2, 4, 0,
COLOR1 and COLOR2 may be either strings containing the color name,
or lists of the form (RED GREEN BLUE), each in the range 0 to 65535 inclusive.
If FRAME is unspecified or nil, the current frame is used.
-If METRIC is unspecified or nil, a modified L*u*v* metric is used. */)
+If METRIC is specified, it should be a function that accepts two
+lists of the form (RED GREEN BLUE). The default metric uses a
+modified L*u*v* space. */)
(Lisp_Object color1, Lisp_Object color2, Lisp_Object frame,
Lisp_Object metric)
{
@@ -4112,7 +4114,15 @@ If METRIC is unspecified or nil, a modified L*u*v* metric is used. */)
if (NILP (metric))
return make_number (color_distance (&cdef1, &cdef2));
else
- return call2 (metric, color1, color2);
+ {
+ Lisp_Object rgb1 = list3 (make_number (&cdef1.red),
+ make_number (&cdef1.green),
+ make_number (&cdef1.blue));
+ Lisp_Object rgb2 = list3 (make_number (&cdef2.red),
+ make_number (&cdef2.green),
+ make_number (&cdef2.blue));
+ return call2 (metric, rgb1, rgb2);
+ }
}
\f
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: Doc string and operation of color-distance
2017-09-14 19:43 ` Mark Oteiza
@ 2017-09-14 20:23 ` Mark Oteiza
2017-09-15 13:41 ` Eli Zaretskii
2017-09-15 13:40 ` Eli Zaretskii
1 sibling, 1 reply; 12+ messages in thread
From: Mark Oteiza @ 2017-09-14 20:23 UTC (permalink / raw
To: Eli Zaretskii; +Cc: emacs-devel
On 14/09/17 at 03:43pm, Mark Oteiza wrote:
> + Lisp_Object rgb1 = list3 (make_number (&cdef1.red),
> + make_number (&cdef1.green),
> + make_number (&cdef1.blue));
> + Lisp_Object rgb2 = list3 (make_number (&cdef2.red),
> + make_number (&cdef2.green),
> + make_number (&cdef2.blue));
I guess these need RGB values to be shifted?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Doc string and operation of color-distance
2017-09-14 19:12 ` Mark Oteiza
@ 2017-09-15 13:30 ` Eli Zaretskii
2017-09-15 16:45 ` Mark Oteiza
0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2017-09-15 13:30 UTC (permalink / raw
To: Mark Oteiza; +Cc: emacs-devel
> Date: Thu, 14 Sep 2017 15:12:26 -0400
> From: Mark Oteiza <mvoteiza@udel.edu>
> Cc: emacs-devel@gnu.org
>
> On 14/09/17 at 09:45pm, Eli Zaretskii wrote:
> > Mark, the new color-distance says something strange in its doc string:
> >
> > (color-distance COLOR1 COLOR2 &optional FRAME METRIC)
> >
> > Return an integer distance between COLOR1 and COLOR2 on FRAME.
> > COLOR1 and COLOR2 may be either strings containing the color name,
> > or lists of the form (RED GREEN BLUE), each in the range 0 to 65535 inclusive.
> > If FRAME is unspecified or nil, the current frame is used.
> > If METRIC is unspecified or nil, a modified L*u*v* metric is used.
> >
> > The last sentence should say "non-nil", I think
>
> No, the 'nil' is correct--if no metric is given as an argument,
> color_distance is used which, as the comments say, is a modified L*u*v*
> metric.
Yes, but the comments also reference the article which describes the
metric, whereas the doc string doesn't.
I think we should either leave the nil case alone -- it wasn't
documented before in such detail -- or add more explanations and
perhaps the reference.
What's important is to describe the non-nil case.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Doc string and operation of color-distance
2017-09-14 19:43 ` Mark Oteiza
2017-09-14 20:23 ` Mark Oteiza
@ 2017-09-15 13:40 ` Eli Zaretskii
2017-09-15 15:15 ` Mark Oteiza
2017-09-15 16:32 ` Mark Oteiza
1 sibling, 2 replies; 12+ messages in thread
From: Eli Zaretskii @ 2017-09-15 13:40 UTC (permalink / raw
To: Mark Oteiza; +Cc: emacs-devel
> From: Mark Oteiza <mvoteiza@udel.edu>
> Date: Thu, 14 Sep 2017 15:43:28 -0400
> Cc: emacs-devel@gnu.org
>
> At the very least we should feed metrics the translated RGBs.
Yes. But it would be nicer if the lcms.c functions accepted colors in
any form supported by Emacs: a color name, a string RGB spec, or a
list of RGB values. I think.
> Beyond that, in order to use (for example) lcms2-cam02-ucs with
> color-distance, one has to translate RGB to XYZ, so the metric would be
>
> (lambda (a b)
> (lcms2-cam02-ucs (color-srgb-to-xyz a) (color-srgb-to-xyz b))
color-srgb-to-xyz accepts 3 arguments, not one. Also, its doc string
says the components are floats between 0 and 1. So this should be
amended accordingly.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Doc string and operation of color-distance
2017-09-14 20:23 ` Mark Oteiza
@ 2017-09-15 13:41 ` Eli Zaretskii
0 siblings, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2017-09-15 13:41 UTC (permalink / raw
To: Mark Oteiza; +Cc: emacs-devel
> Date: Thu, 14 Sep 2017 16:23:12 -0400
> From: Mark Oteiza <mvoteiza@udel.edu>
> Cc: emacs-devel@gnu.org
>
> On 14/09/17 at 03:43pm, Mark Oteiza wrote:
> > + Lisp_Object rgb1 = list3 (make_number (&cdef1.red),
> > + make_number (&cdef1.green),
> > + make_number (&cdef1.blue));
> > + Lisp_Object rgb2 = list3 (make_number (&cdef2.red),
> > + make_number (&cdef2.green),
> > + make_number (&cdef2.blue));
>
> I guess these need RGB values to be shifted?
Why? color_distance only shifts them because it wants to work on
8-bit color components, but you want 16-bit components, AFAIU.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Doc string and operation of color-distance
2017-09-15 13:40 ` Eli Zaretskii
@ 2017-09-15 15:15 ` Mark Oteiza
2017-09-15 16:32 ` Mark Oteiza
1 sibling, 0 replies; 12+ messages in thread
From: Mark Oteiza @ 2017-09-15 15:15 UTC (permalink / raw
To: Eli Zaretskii; +Cc: emacs-devel
On 15/09/17 at 04:40pm, Eli Zaretskii wrote:
> > From: Mark Oteiza <mvoteiza@udel.edu>
> > Date: Thu, 14 Sep 2017 15:43:28 -0400
> > Cc: emacs-devel@gnu.org
> >
> > At the very least we should feed metrics the translated RGBs.
>
> Yes. But it would be nicer if the lcms.c functions accepted colors in
> any form supported by Emacs: a color name, a string RGB spec, or a
> list of RGB values. I think.
>
> > Beyond that, in order to use (for example) lcms2-cam02-ucs with
> > color-distance, one has to translate RGB to XYZ, so the metric would be
> >
> > (lambda (a b)
> > (lcms2-cam02-ucs (color-srgb-to-xyz a) (color-srgb-to-xyz b))
>
> color-srgb-to-xyz accepts 3 arguments, not one. Also, its doc string
> says the components are floats between 0 and 1. So this should be
> amended accordingly.
Ah, yes I forget about color.el's calling conventions…
(defun lcms-rgb16->rgb1 (color)
(mapcar (lambda (x) (/ (lsh x -8) 255.0)) color))
(lambda (a b)
(lcms-cam02-ucs (apply #'color-srgb-to-xyz (color-rgb16b->rgb1 a))
(apply #'color-srgb-to-xyz (color-rgb16b->rgb1 b))))
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Doc string and operation of color-distance
2017-09-15 13:40 ` Eli Zaretskii
2017-09-15 15:15 ` Mark Oteiza
@ 2017-09-15 16:32 ` Mark Oteiza
2017-09-15 17:26 ` Eli Zaretskii
1 sibling, 1 reply; 12+ messages in thread
From: Mark Oteiza @ 2017-09-15 16:32 UTC (permalink / raw
To: Eli Zaretskii; +Cc: emacs-devel
On 15/09/17 at 04:40pm, Eli Zaretskii wrote:
> > From: Mark Oteiza <mvoteiza@udel.edu>
> > Date: Thu, 14 Sep 2017 15:43:28 -0400
> > Cc: emacs-devel@gnu.org
> >
> > At the very least we should feed metrics the translated RGBs.
>
> Yes. But it would be nicer if the lcms.c functions accepted colors in
> any form supported by Emacs: a color name, a string RGB spec, or a
> list of RGB values. I think.
But color-distance and color-values do that already, and it's just
a matter of converting 16bit RGB to another color space, and all those
utilities exist already in color.el
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Doc string and operation of color-distance
2017-09-15 13:30 ` Eli Zaretskii
@ 2017-09-15 16:45 ` Mark Oteiza
2017-09-15 17:24 ` Eli Zaretskii
0 siblings, 1 reply; 12+ messages in thread
From: Mark Oteiza @ 2017-09-15 16:45 UTC (permalink / raw
To: Eli Zaretskii; +Cc: emacs-devel
On 15/09/17 at 04:30pm, Eli Zaretskii wrote:
> > Date: Thu, 14 Sep 2017 15:12:26 -0400
> > From: Mark Oteiza <mvoteiza@udel.edu>
> > Cc: emacs-devel@gnu.org
> >
> > On 14/09/17 at 09:45pm, Eli Zaretskii wrote:
> > > Mark, the new color-distance says something strange in its doc string:
> > >
> > > (color-distance COLOR1 COLOR2 &optional FRAME METRIC)
> > >
> > > Return an integer distance between COLOR1 and COLOR2 on FRAME.
> > > COLOR1 and COLOR2 may be either strings containing the color name,
> > > or lists of the form (RED GREEN BLUE), each in the range 0 to 65535 inclusive.
> > > If FRAME is unspecified or nil, the current frame is used.
> > > If METRIC is unspecified or nil, a modified L*u*v* metric is used.
> > >
> > > The last sentence should say "non-nil", I think
> >
> > No, the 'nil' is correct--if no metric is given as an argument,
> > color_distance is used which, as the comments say, is a modified L*u*v*
> > metric.
>
> Yes, but the comments also reference the article which describes the
> metric, whereas the doc string doesn't.
>
> I think we should either leave the nil case alone -- it wasn't
> documented before in such detail -- or add more explanations and
> perhaps the reference.
>
> What's important is to describe the non-nil case.
OK, I agree. How about the following?
diff --git a/src/xfaces.c b/src/xfaces.c
index 012de4e7af..b309c16127 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -4093,7 +4093,8 @@ DEFUN ("color-distance", Fcolor_distance, Scolor_distance, 2, 4, 0,
COLOR1 and COLOR2 may be either strings containing the color name,
or lists of the form (RED GREEN BLUE), each in the range 0 to 65535 inclusive.
If FRAME is unspecified or nil, the current frame is used.
-If METRIC is unspecified or nil, a modified L*u*v* metric is used. */)
+If METRIC is specified, it should be a function that accepts
+two lists of the form (RED GREEN BLUE) aforementioned. */)
(Lisp_Object color1, Lisp_Object color2, Lisp_Object frame,
Lisp_Object metric)
{
@@ -4112,7 +4113,13 @@ If METRIC is unspecified or nil, a modified L*u*v* metric is used. */)
if (NILP (metric))
return make_number (color_distance (&cdef1, &cdef2));
else
- return call2 (metric, color1, color2);
+ return call2 (metric,
+ list3 (make_number (cdef1.red),
+ make_number (cdef1.green),
+ make_number (cdef1.blue)),
+ list3 (make_number (cdef2.red),
+ make_number (cdef2.green),
+ make_number (cdef2.blue)));
}
\f
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: Doc string and operation of color-distance
2017-09-15 16:45 ` Mark Oteiza
@ 2017-09-15 17:24 ` Eli Zaretskii
0 siblings, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2017-09-15 17:24 UTC (permalink / raw
To: Mark Oteiza; +Cc: emacs-devel
> Date: Fri, 15 Sep 2017 12:45:43 -0400
> From: Mark Oteiza <mvoteiza@udel.edu>
> Cc: emacs-devel@gnu.org
>
> > I think we should either leave the nil case alone -- it wasn't
> > documented before in such detail -- or add more explanations and
> > perhaps the reference.
> >
> > What's important is to describe the non-nil case.
>
> OK, I agree. How about the following?
LGTM, thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Doc string and operation of color-distance
2017-09-15 16:32 ` Mark Oteiza
@ 2017-09-15 17:26 ` Eli Zaretskii
0 siblings, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2017-09-15 17:26 UTC (permalink / raw
To: Mark Oteiza; +Cc: emacs-devel
> Date: Fri, 15 Sep 2017 12:32:09 -0400
> From: Mark Oteiza <mvoteiza@udel.edu>
> Cc: emacs-devel@gnu.org
>
> > Yes. But it would be nicer if the lcms.c functions accepted colors in
> > any form supported by Emacs: a color name, a string RGB spec, or a
> > list of RGB values. I think.
>
> But color-distance and color-values do that already, and it's just
> a matter of converting 16bit RGB to another color space, and all those
> utilities exist already in color.el
If we don't intend that lcms.c functions be used independently, then I
guess it's OK.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-09-15 17:26 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-14 18:45 Doc string and operation of color-distance Eli Zaretskii
2017-09-14 19:12 ` Mark Oteiza
2017-09-15 13:30 ` Eli Zaretskii
2017-09-15 16:45 ` Mark Oteiza
2017-09-15 17:24 ` Eli Zaretskii
2017-09-14 19:43 ` Mark Oteiza
2017-09-14 20:23 ` Mark Oteiza
2017-09-15 13:41 ` Eli Zaretskii
2017-09-15 13:40 ` Eli Zaretskii
2017-09-15 15:15 ` Mark Oteiza
2017-09-15 16:32 ` Mark Oteiza
2017-09-15 17:26 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).