Bug 764181. Keep the output of the TRC between 0 and 1. r=bgirard
authorJeff Muizelaar <jmuizelaar@mozilla.com>
Thu, 07 Jun 2012 16:47:34 -0400
changeset 96527 a41fd66f124530dc07856ea67da4067b843f268b
parent 96526 bc7286e373c9ddea9fe819473e25e5b05e91cfd3
child 96528 5387220c0609850b533bba269ed449d4b8f3eeb6
push id10581
push userjmuizelaar@mozilla.com
push dateTue, 12 Jun 2012 22:17:41 +0000
treeherdermozilla-inbound@a41fd66f1245 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgirard
bugs764181
milestone16.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 764181. Keep the output of the TRC between 0 and 1. r=bgirard 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.
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;
 }