Make color-distance symmetric and more accurate

* src/xfaces.c (color_distance): Don't throw away the low 8 bits of
the colours, and make the function symmetric (bug41544)
(Fcolor_distance): Add caution about this not being a true metric.
* test/src/xfaces-tests.el: New file.

2 files changed, 40 insertions, 11 deletions

diff --git a/src/xfaces.c b/src/xfaces.c
index 7d7aff95c11..cf155288bd1 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -4356,15 +4356,15 @@ color_distance (Emacs_Color *x, Emacs_Color *y)
 
      See <https://www.compuphase.com/cmetric.htm> for more info.  */
 
-  long r = (x->red   - y->red)   >> 8;
-  long g = (x->green - y->green) >> 8;
-  long b = (x->blue  - y->blue)  >> 8;
-  long r_mean = (x->red + y->red) >> 9;
-
-  return
-    (((512 + r_mean) * r * r) >> 8)
-    + 4 * g * g
-    + (((767 - r_mean) * b * b) >> 8);
+  long long r = x->red   - y->red;
+  long long g = x->green - y->green;
+  long long b = x->blue  - y->blue;
+  long long r_mean = (x->red + y->red) >> 1;
+
+  return (((((2 * 65536 + r_mean) * r * r) >> 16)
+           + 4 * g * g
+           + (((2 * 65536 + 65535 - r_mean) * b * b) >> 16))
+          >> 16);
 }
 
 
@@ -4374,7 +4374,9 @@ 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 specified, it should be a function that accepts
-two lists of the form (RED GREEN BLUE) aforementioned. */)
+two lists of the form (RED GREEN BLUE) aforementioned.
+Despite the name, this is not a true distance metric as it does not satisfy
+the triangle inequality.  */)
   (Lisp_Object color1, Lisp_Object color2, Lisp_Object frame,
    Lisp_Object metric)
 {
@@ -4931,7 +4933,7 @@ DEFUN ("face-attributes-as-vector", Fface_attributes_as_vector,
 
 /* If the distance (as returned by color_distance) between two colors is
    less than this, then they are considered the same, for determining
-   whether a color is supported or not.  The range of values is 0-65535.  */
+   whether a color is supported or not.  */
 
 #define TTY_SAME_COLOR_THRESHOLD  10000
 
diff --git a/test/src/xfaces-tests.el b/test/src/xfaces-tests.el
new file mode 100644
index 00000000000..f08a87a518a
--- /dev/null
+++ b/test/src/xfaces-tests.el
@@ -0,0 +1,27 @@
+;;; xfaces-tests.el --- tests for xfaces.c           -*- lexical-binding: t -*-
+
+;; Copyright (C) 2020 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+(require 'ert)
+
+(ert-deftest xfaces-color-distance ()
+  ;; Check symmetry (bug#51455).
+  (should (equal (color-distance "#222222" "#ffffff")
+                 (color-distance "#ffffff" "#222222"))))
+
+(provide 'xfaces-tests)