# HG changeset patch
# User Jeff Muizelaar <jmuizelaar@mozilla.com>
# Date 1339102054 14400
# Node ID d53fd484df985b283b6a310416c386bdd27f4b26
# Parent  181fd83fcdfba1ae3702603294e0fdb4ade82a17
Bug 764181. Keep the output of the TRC between 0 and 1. r=bgirard,a=lsblakk

Section 10.16: The domain and range of each function shall be [0,0 1,0].
Any function value outside the range shall be clipped to the range of the function.

target: ESR unknown

diff --git a/gfx/qcms/transform_util.c b/gfx/qcms/transform_util.c
--- a/gfx/qcms/transform_util.c
+++ b/gfx/qcms/transform_util.c
@@ -10,26 +10,28 @@
 #if !defined(INFINITY)
 #define INFINITY HUGE_VAL
 #endif
 
 #define PARAMETRIC_CURVE_TYPE 0x70617261 //'para'
 
 /* value must be a value between 0 and 1 */
 //XXX: is the above a good restriction to have?
-float lut_interp_linear(double value, uint16_t *table, int length)
+// the output range of this functions is 0..1
+float lut_interp_linear(double input_value, uint16_t *table, int length)
 {
 	int upper, lower;
-	value = value * (length - 1); // scale to length of the array
-	upper = ceil(value);
-	lower = floor(value);
+	float value;
+	input_value = input_value * (length - 1); // scale to length of the array
+	upper = ceil(input_value);
+	lower = floor(input_value);
 	//XXX: can we be more performant here?
-	value = table[upper]*(1. - (upper - value)) + table[lower]*(upper - value);
+	value = table[upper]*(1. - (upper - input_value)) + table[lower]*(upper - input_value);
 	/* scale the value */
-	return value * (1./65535.);
+	return value * (1.f/65535.f);
 }
 
 /* same as above but takes and returns a uint16_t value representing a range from 0..1 */
 uint16_t lut_interp_linear16(uint16_t input_value, uint16_t *table, int length)
 {
 	/* Start scaling input_value to the length of the array: 65535*(length-1).
 	 * We'll divide out the 65535 next */
 	uint32_t value = (input_value * (length - 1));
@@ -94,21 +96,23 @@ uint16_t lut_interp_linear16(uint16_t in
 	uint32_t interp = value % 4096;
 
 	value = (table[upper]*(interp) + table[lower]*(4096 - interp))/4096; // 0..4096*4096
 
 	return value;
 }
 #endif
 
-void compute_curve_gamma_table_type1(float gamma_table[256], double gamma)
+void compute_curve_gamma_table_type1(float gamma_table[256], uint16_t gamma)
 {
 	unsigned int i;
+	float gamma_float = u8Fixed8Number_to_float(gamma);
 	for (i = 0; i < 256; i++) {
-		gamma_table[i] = pow(i/255., gamma);
+                // 0..1^(0..255 + 255/256) will always be between 0 and 1
+		gamma_table[i] = pow(i/255., gamma_float);
 	}
 }
 
 void compute_curve_gamma_table_type2(float gamma_table[256], uint16_t *table, int length)
 {
 	unsigned int i;
 	for (i = 0; i < 256; i++) {
 		gamma_table[i] = lut_interp_linear(i/255., table, length);
@@ -165,40 +169,51 @@ void compute_curve_gamma_table_type_para
                 f = 0;
                 interval = -INFINITY;
         }       
         for (X = 0; X < 256; X++) {
                 if (X >= interval) {
                         // XXX The equations are not exactly as definied in the spec but are
                         //     algebraic equivilent.
                         // TODO Should division by 255 be for the whole expression.
-                        gamma_table[X] = pow(a * X / 255. + b, y) + c + e;
+                        gamma_table[X] = clamp_float(pow(a * X / 255. + b, y) + c + e);
                 } else {
-                        gamma_table[X] = c * X / 255. + f;
+                        gamma_table[X] = clamp_float(c * X / 255. + f);
                 }
         }
 }
 
 void compute_curve_gamma_table_type0(float gamma_table[256])
 {
 	unsigned int i;
 	for (i = 0; i < 256; i++) {
 		gamma_table[i] = i/255.;
 	}
 }
 
-
 float clamp_float(float a)
 {
-        if (a > 1.)
-                return 1.;
-        else if (a < 0)
-                return 0;
-        else
-                return a;
+	/* One would naturally write this function as the following:
+	if (a > 1.)
+		return 1.;
+	else if (a < 0)
+		return 0;
+	else
+		return a;
+
+	However, that version will let NaNs pass through which is undesirable
+	for most consumers.
+	*/
+
+	if (a > 1.)
+		return 1.;
+	else if (a >= 0)
+		return a;
+	else // a < 0 or a is NaN
+		return 0;
 }
 
 unsigned char clamp_u8(float v)
 {
 	if (v > 255.)
 		return 255;
 	else if (v < 0)
 		return 0;
@@ -222,17 +237,17 @@ float *build_input_gamma_table(struct cu
 	gamma_table = malloc(sizeof(float)*256);
 	if (gamma_table) {
 		if (TRC->type == PARAMETRIC_CURVE_TYPE) {
 			compute_curve_gamma_table_type_parametric(gamma_table, TRC->parameter, TRC->count);
 		} else {
 			if (TRC->count == 0) {
 				compute_curve_gamma_table_type0(gamma_table);
 			} else if (TRC->count == 1) {
-				compute_curve_gamma_table_type1(gamma_table, u8Fixed8Number_to_float(TRC->data[0]));
+				compute_curve_gamma_table_type1(gamma_table, TRC->data[0]);
 			} else {
 				compute_curve_gamma_table_type2(gamma_table, TRC->data, TRC->count);
 			}
 		}
 	}
         return gamma_table;
 }
 
