Fix #45 Fix attrocious CMC implementation + add access to parameters

DESCRIPTION

@@ -27,5 +27,5 @@ Suggests:
     testthat (>= 3.0.0)
 Encoding: UTF-8
 Roxygen: list(markdown=TRUE)
-RoxygenNote: 7.2.0
+RoxygenNote: 7.3.1
 Config/testthat/edition: 3

R/compare_colour.R

@@ -1,56 +1,60 @@
 #' Calculate the distance between colours
-#' 
-#' There are many ways to measure the distance between colours. `farver` 
-#' provides 5 different algorithms, ranging from simple euclidean distance in 
+#'
+#' There are many ways to measure the distance between colours. `farver`
+#' provides 5 different algorithms, ranging from simple euclidean distance in
 #' RGB space, to different perceptual measures such as CIE2000.
-#' 
+#'
 #' @inheritSection  convert_colour Handling of non-finite and out of bounds values
-#' 
-#' @param from,to Numeric matrices with colours to compare - the format is the 
-#' same as that for [convert_colour()]. If `to` is not set `from` will be 
+#'
+#' @param from,to Numeric matrices with colours to compare - the format is the
+#' same as that for [convert_colour()]. If `to` is not set `from` will be
 #' compared with itself and only the upper triangle will get calculated
-#' 
-#' @param from_space,to_space The colour space of `from` and `to` respectively. 
+#'
+#' @param from_space,to_space The colour space of `from` and `to` respectively.
 #' `to_space` defaults to be the same as `from_space`.
-#' 
-#' @param method The method to use for comparison. Either `'euclidean'`, 
+#'
+#' @param method The method to use for comparison. Either `'euclidean'`,
 #' `'cie1976'`, `'cie94'`, `'cie2000'`, or `'cmc'`
-#' 
-#' @param white_from,white_to The white reference of the from and to colour 
-#' space. Will only have an effect for relative colour spaces such as Lab and 
+#'
+#' @param white_from,white_to The white reference of the from and to colour
+#' space. Will only have an effect for relative colour spaces such as Lab and
 #' luv. Any value accepted by [as_white_ref()] allowed.
-#' 
+#'
+#' @param lightness,chroma Weight of lightness vs chroma when using CMC. Common
+#' values are `2` and `1` (default) for acceptability and `1` and `1` for
+#' imperceptibility
+#'
 #' @return A numeric matrix with the same number of rows as colours in `from`
 #' and the same number of columns as colours in `to`. If `to` is not given, only
 #' the upper triangle will be returned.
-#' 
+#'
 #' @export
-#' 
-#' @examples 
+#'
+#' @examples
 #' r <- decode_colour(rainbow(10))
 #' h <- decode_colour(heat.colors(15))
-#' 
+#'
 #' # Compare two sets of colours
 #' compare_colour(r, h, 'rgb', method = 'cie2000')
-#' 
+#'
 #' # Compare a set of colours with itself
 #' compare_colour(r, from_space = 'rgb', method = 'cmc')
-#' 
+#'
 #' # Compare colours from different colour spaces
 #' h_luv <- convert_colour(h, 'rgb', 'luv')
 #' compare_colour(r, h_luv, 'rgb', 'luv')
-#' 
-compare_colour <- function(from, to = NULL, from_space, to_space = from_space, method = 'euclidean', white_from = 'D65', white_to = white_from) {
+#'
+compare_colour <- function(from, to = NULL, from_space, to_space = from_space, method = 'euclidean', white_from = 'D65', white_to = white_from, lightness = 2, chroma = 1) {
   sym <- FALSE
   if (is.null(to)) {
     to <- from;
     sym <- TRUE
   }
-  compare_c(from, to, colourspace_match(from_space), colourspace_match(to_space), distance_match(method), sym, as_white_ref(white_from), as_white_ref(white_to))
+  compare_c(from, to, colourspace_match(from_space), colourspace_match(to_space), distance_match(method), sym, as_white_ref(white_from), as_white_ref(white_to), lightness, chroma)
 }
 
-compare_c <- function(from, to, from_space, to_space, method, symmetric, white_from, white_to) {
-  .Call(`farver_compare_c`, as.matrix(from), as.matrix(to), as.integer(from_space), 
-        as.integer(to_space), as.integer(method), as.logical(symmetric), 
-        as.numeric(white_from), as.numeric(white_to))
+compare_c <- function(from, to, from_space, to_space, method, symmetric, white_from, white_to, lightness, chroma) {
+  .Call(`farver_compare_c`, as.matrix(from), as.matrix(to), as.integer(from_space),
+        as.integer(to_space), as.integer(method), as.logical(symmetric),
+        as.numeric(white_from), as.numeric(white_to), as.numeric(lightness), as.numeric(chroma))
 }

src/Comparison.cpp

@@ -147,29 +147,36 @@ namespace ColorSpace {
 		return sqrt(SQR(deltaL / sl) + SQR(deltaC / sc) + SQR(deltaH / sh) + rt * deltaC / sc * deltaH / sh);
 	}
 
-
-	const double CmcComparison::defaultLightness = 2.;
-	const double CmcComparison::defaultChroma = 1.;
+  double CmcComparison::defaultLightness = 2.0;
+  double CmcComparison::defaultChroma = 1.0;
 	double CmcComparison::Compare(IColorSpace *a, IColorSpace *b) {
+	  static const double pi = 3.141592653589793115998;
 	  if (!a->valid || !b->valid) return -1.0;
 		Lch lch_a;
 		Lch lch_b;
+		Lab lab_a;
+		Lab lab_b;
 
 		a->To<Lch>(&lch_a);
 		b->To<Lch>(&lch_b);
+		a->To<Lab>(&lab_a);
+		b->To<Lab>(&lab_b);
 
-		double deltaL = lch_a.l - lch_b.l;
-		double deltaC = lch_a.c - lch_b.c;
-		double deltaH = 0;
+		double sl = (lch_a.l < 16) ? 0.511 : (0.040975*lch_a.l / (1 + 0.01765*lch_a.l));
+		double sc = 0.0638*lch_a.c / (1 + 0.0131*lch_a.c) + 0.638;
 
+		double t = (164 <= lch_a.h && lch_a.h <= 345) ? (0.56 + std::abs(0.2*std::cos(pi * (lch_a.h + 168) / 180.0))) : (0.36 + std::abs(0.4*std::cos(pi * (lch_a.h + 35) / 180.0)));
 		double f = std::sqrt(POW4(lch_a.c) / (POW4(lch_a.c) + 1900));
-		double t = (164 <= lch_a.h && lch_a.h <= 345) ? (0.56 + std::abs(0.2*std::cos(lch_a.h + 168))) : (0.36 + std::abs(0.4*std::cos(lch_a.h + 35)));
 
-		double sl = (lch_a.l < 16) ? 0.511 : (0.040975*lch_a.l / (1 + 0.01765*lch_a.l));
-		double sc = 0.0638*lch_a.c / (1 + 0.0131*lch_a.c) + 0.638;
 		double sh = sc*(f*t + 1 - f);
 
-		return std::sqrt(SQR(deltaL / (defaultLightness*sl)) + SQR(deltaC / (defaultChroma*sc)) + SQR(deltaH / sh));
+		double deltaL = lch_a.l - lch_b.l;
+		double deltaC = lch_a.c - lch_b.c;
+    double deltaA = lab_a.a - lab_b.a;
+    double deltaB = lab_a.b - lab_b.b;
+    double deltaH2 = deltaA * deltaA + deltaB * deltaB - deltaC * deltaC;
+
+		return std::sqrt(SQR(deltaL / (defaultLightness*sl)) + SQR(deltaC / (defaultChroma*sc)) + deltaH2 / SQR(sh));
 	}
 }
 

src/Comparison.h

@@ -36,8 +36,8 @@ namespace ColorSpace {
 
 
 	struct CmcComparison {
-		static const double defaultLightness;
-		static const double defaultChroma;
+		static double defaultLightness;
+		static double defaultChroma;
 		static double Compare(IColorSpace *a, IColorSpace *b);
 	};
 }

src/farver.cpp

@@ -264,6 +264,8 @@ SEXP compare_dispatch_from(SEXP from, SEXP to, int from_space, int to_space, int
   return from;
 }
 
-SEXP compare_c(SEXP from, SEXP to, SEXP from_space, SEXP to_space, SEXP dist, SEXP sym, SEXP white_from, SEXP white_to) {
+SEXP compare_c(SEXP from, SEXP to, SEXP from_space, SEXP to_space, SEXP dist, SEXP sym, SEXP white_from, SEXP white_to, SEXP lightness, SEXP chroma) {
+  ColorSpace::CmcComparison::defaultLightness = REAL(lightness)[0];
+  ColorSpace::CmcComparison::defaultChroma = REAL(chroma)[0];
   return compare_dispatch_from(from, to, INTEGER(from_space)[0], INTEGER(to_space)[0], INTEGER(dist)[0], LOGICAL(sym)[0], white_from, white_to);
 }

src/farver.h

@@ -7,7 +7,7 @@
 #include <Rinternals.h>
 
 // these are used in the dispatcher functions
-// this is one-based in the same order as the `colourspaces` 
+// this is one-based in the same order as the `colourspaces`
 // vector define in aaa.R
 #define CMY 1
 #define CMYK 2
@@ -32,7 +32,7 @@
 #define CMC 5
 
 SEXP convert_c(SEXP colour, SEXP from, SEXP to, SEXP white_from, SEXP white_to);
-SEXP compare_c(SEXP from, SEXP to, SEXP from_space, SEXP to_space, SEXP dist, SEXP sym, SEXP white_from, SEXP white_to);
+SEXP compare_c(SEXP from, SEXP to, SEXP from_space, SEXP to_space, SEXP dist, SEXP sym, SEXP white_from, SEXP white_to, SEXP lightness, SEXP chroma);
 
 inline void copy_names(SEXP from, SEXP to) {
   SEXP names;
@@ -45,7 +45,7 @@ inline void copy_names(SEXP from, SEXP to) {
   } else {
     names = PROTECT(Rf_getAttrib(from, R_NamesSymbol));
   }
-  
+
   if (!Rf_isNull(names)) {
     if (Rf_isMatrix(to)) {
       SEXP dn = PROTECT(Rf_allocVector(VECSXP, 2));
@@ -56,7 +56,7 @@ inline void copy_names(SEXP from, SEXP to) {
       Rf_namesgets(to, names);
     }
   }
-  
+
   UNPROTECT(1);
 }
 
@@ -80,7 +80,7 @@ inline void copy_names(SEXP from1, SEXP from2, SEXP to) {
   } else {
     names2 = PROTECT(Rf_getAttrib(from2, Rf_install("names")));
   }
-  
+
   if ((!Rf_isNull(names1) || !Rf_isNull(names2)) && Rf_isMatrix(to)) {
     names = PROTECT(Rf_allocVector(VECSXP, 2));
     if (!Rf_isNull(names1)) {
@@ -92,7 +92,7 @@ inline void copy_names(SEXP from1, SEXP from2, SEXP to) {
     Rf_setAttrib(to, Rf_install("dimnames"), names);
     UNPROTECT(1);
   }
-  
+
   UNPROTECT(2);
 }
 
@@ -134,7 +134,7 @@ inline void fill_rgb<ColorSpace::Cmyk>(ColorSpace::Rgb* rgb, int x1, int x2, int
 }
 
 // these grab values from the Space type and use them to fill `row`
-// unfortunately, given how the `ColorSpace` C++ library is written, 
+// unfortunately, given how the `ColorSpace` C++ library is written,
 // this has to do lots of special casing
 template <typename Space>
 inline void grab(const Space&, double* x1, double* x2, double* x3, double* x4) ;

src/init.cpp

@@ -14,7 +14,7 @@ ColourMap& get_named_colours() {
 
 static const R_CallMethodDef CallEntries[] = {
   {"farver_convert_c", (DL_FUNC) &convert_c, 5},
-  {"farver_compare_c", (DL_FUNC) &compare_c, 8},
+  {"farver_compare_c", (DL_FUNC) &compare_c, 10},
   {"farver_encode_c", (DL_FUNC) &encode_c, 4},
   {"farver_decode_c", (DL_FUNC) &decode_c, 5},
   {"farver_encode_channel_c", (DL_FUNC) &encode_channel_c, 7},
@@ -28,10 +28,10 @@ static const R_CallMethodDef CallEntries[] = {
 extern "C" void R_init_farver(DllInfo *dll) {
   R_registerRoutines(dll, NULL, CallEntries, NULL, NULL);
   R_useDynamicSymbols(dll, FALSE);
-  
+
   named_colours = new ColourMap();
 }
 
 extern "C" void R_unload_farver(DllInfo *dll) {
   delete named_colours;
-}
+}