bug#28400: 26.0.50; lcms2 bindings

[Eli Zaretskii]

> Date: Mon, 11 Sep 2017 19:10:06 -0400
> From: Mark Oteiza <mvoteiza@udel.edu>
> Cc: 28400@debbugs.gnu.org
> 
> > Oh, I see.  But in that case, I think adding a new file and the
> > configure.ac business is actually quite easy.
> 
> Humble beginnings attached.

Thanks, it looks good.  A few comments:

> diff --git a/src/lcms.c b/src/lcms.c
> new file mode 100644
> index 0000000000..560b262818
> --- /dev/null
> +++ b/src/lcms.c
> @@ -0,0 +1,157 @@
> +/* Interface to Little CMS
> +   Copyright (C) 2017 Mark Oteiza <mvoteiza@udel.edu>
> +
> +This file is NOT part of GNU Emacs.

This will have to change to our standard preamble, of course.

> +DEFUN ("lcms-cie-de2000", Flcms_cie_de2000, Slcms_cie_de2000, 2, 2, 0,
> +       doc: /* Compute CIEDE2000 metric distance between COLOR1 and COLOR2.
> +Each color is a list of L*a*b* coordinates, where the L* channel ranges from
> +0 to 100, and the a* and b* channels range from -128 to 128.
> +Optional arguments KL, KC, KH are weighting parameters for lightness,
> +chroma, and hue, respectively. */)
> +  (Lisp_Object color1, Lisp_Object color2)

I don't see any optional arguments.

> +DEFUN ("lcms-cam02-ucs", Flcms-cam02-ucs, Slcms_cam02_ucs, 2, 2, 0,
> +       doc: /* Compute CAM02-UCS metric distance between COLOR1 and COLOR2.
> +Each color is a list of XYZ coordinates.
> +Optional argument is the XYZ white point, which defaults to D65. */)
> +  (Lisp_Object color1, Lisp_Object color2)

Likewise.

> +  k = 1.0f / (1.0f + (5.0f * vc.La));

Any reason why you use float constants, as opposed to double?

> +  FL = pow(k, 4) * vc.La +
> +    0.1f * pow(1 - pow(k, 4), 2) * pow(5.0f * vc.La, 1.0f / 3.0f);
> +  Mp1 = 43.86f * log(1.0f + 0.0228f * (jch1.C * pow(FL, 0.25)));
> +  Mp2 = 43.86f * log(1.0f + 0.0228f * (jch2.C * pow(FL, 0.25)));
> +  Jp1 = 1.7f * jch1.J / (1.0f + (0.007f * jch1.J));
> +  Jp2 = 1.7f * jch2.J / (1.0f + (0.007f * jch2.J));
> +  ap1 = Mp1 * cos(jch1.h);
> +  ap2 = Mp2 * cos(jch2.h);
> +  bp1 = Mp1 * sin(jch1.h);
> +  bp2 = Mp2 * sin(jch2.h);
> +  return make_float(sqrt(pow(Jp2 - Jp1, 2) +
> +                         pow(ap2 - ap1, 2) +
> +                         pow(bp2 - bp1, 2)));

I generally dislike 'pow', where simpler functions will do.  'pow' is
expensive and less accurate numerically than the alternatives (where
they exist); it also makes the code slightly harder to read and
understand.  Granted, this code is unlikely to run in the inner-most
loops of some Lisp program, or to require last-ulp accuracy, so some
of these arguments are admittedly weak.  But still...

So I'd replace:

   pow (SOMETHING, 2)             with SOMETHING*SOMETHING
   pow (SOMETHING, 1./3.)         with cubrt (SOMETHING)
   pow (k, 4)                     with k * k * k * k
   pow (FL, 0.25)                 with sqrt (sqrt (FL))

etc.

Also, a nit: we leave a blank between the function name and the
following left paren.

I think this will also need a NEWS entry, under "Installation
Changes".

Thanks again for working on this.