From: Steinar H. Gunderson <sgunderson@bigfoot.com>
Date: Wed, 1 Jan 2014 11:43:36 +0000 (+0100)
Subject: Calculate the RGB-to-XYZ matrix ourselves instead of using a “magic” one from Wikipedia.
X-Git-Tag: 1.0~100
X-Git-Url: https://git.sesse.net/?p=movit;a=commitdiff_plain;h=ddce0987c54cd04fdbb2aee2886ea982e13b84f2

Calculate the RGB-to-XYZ matrix ourselves instead of using a “magic” one from Wikipedia.

Generally we try to derive such values from first principles when possible;
this is some of the oldest code in Movit, which explains why it was forgotten.

Maybe longer-term we should move this out of ColorspaceConversionEffect into a
free function in util.h or something similar, but for now, this will do.
---

diff --git a/colorspace_conversion_effect.cpp b/colorspace_conversion_effect.cpp
index bb23afc..bd4f70e 100644
--- a/colorspace_conversion_effect.cpp
+++ b/colorspace_conversion_effect.cpp
@@ -30,7 +30,7 @@ ColorspaceConversionEffect::ColorspaceConversionEffect()
 	register_int("destination_space", (int *)&destination_space);
 }
 
-Matrix3d get_xyz_matrix(Colorspace space)
+Matrix3d ColorspaceConversionEffect::get_xyz_matrix(Colorspace space)
 {
 	if (space == COLORSPACE_XYZ) {
 		return Matrix3d::Identity();
diff --git a/colorspace_conversion_effect.h b/colorspace_conversion_effect.h
index 8e691c4..b21f72e 100644
--- a/colorspace_conversion_effect.h
+++ b/colorspace_conversion_effect.h
@@ -9,6 +9,7 @@
 // will simply stay out-of-gamut, and probably clip in the output stage.
 
 #include <string>
+#include <Eigen/Core>
 
 #include "effect.h"
 #include "image_format.h"
@@ -26,6 +27,9 @@ public:
 	virtual bool needs_srgb_primaries() const { return false; }
 	virtual AlphaHandling alpha_handling() const { return DONT_CARE_ALPHA_TYPE; }
 
+	// Get a conversion matrix from the given color space to XYZ.
+	static Eigen::Matrix3d get_xyz_matrix(Colorspace space);
+
 private:
 	Colorspace source_space, destination_space;
 };
diff --git a/white_balance_effect.cpp b/white_balance_effect.cpp
index a52af6a..2c99ca8 100644
--- a/white_balance_effect.cpp
+++ b/white_balance_effect.cpp
@@ -3,6 +3,7 @@
 #include <GL/glew.h>
 #include <assert.h>
 
+#include "colorspace_conversion_effect.h"
 #include "d65.h"
 #include "effect_util.h"
 #include "util.h"
@@ -38,13 +39,6 @@ Vector3d convert_color_temperature_to_xyz(float T)
 	return Vector3d(x, y, 1.0 - x - y);
 }
 
-// Assuming sRGB primaries, from Wikipedia.
-const double rgb_to_xyz_matrix[9] = {
-	0.4124, 0.2126, 0.0193, 
-	0.3576, 0.7152, 0.1192,
-	0.1805, 0.0722, 0.9505,
-};
-
 /*
  * There are several different perceptual color spaces with different intended
  * uses; for instance, CIECAM02 uses one space (CAT02) for purposes of computing
@@ -116,8 +110,9 @@ std::string WhiteBalanceEffect::output_fragment_shader()
 
 void WhiteBalanceEffect::set_gl_state(GLuint glsl_program_num, const std::string &prefix, unsigned *sampler_num)
 {
+	Matrix3d rgb_to_xyz_matrix = ColorspaceConversionEffect::get_xyz_matrix(COLORSPACE_sRGB);
 	Vector3d rgb(neutral_color.r, neutral_color.g, neutral_color.b);
-	Vector3d xyz = Map<const Matrix3d>(rgb_to_xyz_matrix) * rgb;
+	Vector3d xyz = rgb_to_xyz_matrix * rgb;
 	Vector3d lms_scale = compute_lms_scaling_factors(xyz);
 
 	/*
@@ -144,10 +139,10 @@ void WhiteBalanceEffect::set_gl_state(GLuint glsl_program_num, const std::string
 	 * has to be the opposite of the execution order.
 	 */
 	Matrix3d corr_matrix =
-		Map<const Matrix3d>(rgb_to_xyz_matrix).inverse() *
+		rgb_to_xyz_matrix.inverse() *
 		Map<const Matrix3d>(xyz_to_lms_matrix).inverse() *
 		lms_scale.asDiagonal() *
 		Map<const Matrix3d>(xyz_to_lms_matrix) *
-		Map<const Matrix3d>(rgb_to_xyz_matrix);
+		rgb_to_xyz_matrix;
 	set_uniform_mat3(glsl_program_num, prefix, "correction_matrix", corr_matrix);
 }