Convert XYZ values from PNGs to D50
Previously, we were storing whatever values we got out of the file
and pretending they were D50. Brian has demonstrated that this
doesn't really work.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2180483003
Review-Url: https://codereview.chromium.org/2180483003
diff --git a/src/codec/SkPngCodec.cpp b/src/codec/SkPngCodec.cpp
index 32308b2..ee72af5 100644
--- a/src/codec/SkPngCodec.cpp
+++ b/src/codec/SkPngCodec.cpp
@@ -13,6 +13,7 @@
#include "SkMath.h"
#include "SkOpts.h"
#include "SkPngCodec.h"
+#include "SkPoint3.h"
#include "SkSize.h"
#include "SkStream.h"
#include "SkSwizzler.h"
@@ -178,6 +179,62 @@
0.1430f, 0.0606f, 0.7139f, // * B
};
+static bool convert_to_D50(SkMatrix44* toXYZD50, float toXYZ[9], float whitePoint[2]) {
+ float wX = whitePoint[0];
+ float wY = whitePoint[1];
+ if (wX < 0.0f || wY < 0.0f || (wX + wY > 1.0f)) {
+ return false;
+ }
+
+ // Calculate the XYZ illuminant. Call this the src illuminant.
+ float wZ = 1.0f - wX - wY;
+ float scale = 1.0f / wY;
+ // TODO (msarett):
+ // What are common src illuminants? I'm guessing we will almost always see D65. Should
+ // we go ahead and save a precomputed D65->D50 Bradford matrix? Should we exit early if
+ // if the src illuminant is D50?
+ SkVector3 srcXYZ = SkVector3::Make(wX * scale, 1.0f, wZ * scale);
+
+ // The D50 illuminant.
+ SkVector3 dstXYZ = SkVector3::Make(0.96422f, 1.0f, 0.82521f);
+
+ // Calculate the chromatic adaptation matrix. We will use the Bradford method, thus
+ // the matrices below. The Bradford method is used by Adobe and is widely considered
+ // to be the best.
+ // http://www.brucelindbloom.com/index.html?Eqn_ChromAdapt.html
+ SkMatrix mA, mAInv;
+ mA.setAll(0.8951f, 0.2664f, -0.1614f, -0.7502f, 1.7135f, 0.0367f, 0.0389f, -0.0685f, 1.0296f);
+ mAInv.setAll(0.9869929f, -0.1470543f, 0.1599627f, 0.4323053f, 0.5183603f, 0.0492912f,
+ -0.0085287f, 0.0400428f, 0.9684867f);
+
+ // Map illuminant into cone response domain.
+ SkVector3 srcCone;
+ srcCone.fX = mA[0] * srcXYZ.fX + mA[1] * srcXYZ.fY + mA[2] * srcXYZ.fZ;
+ srcCone.fY = mA[3] * srcXYZ.fX + mA[4] * srcXYZ.fY + mA[5] * srcXYZ.fZ;
+ srcCone.fZ = mA[6] * srcXYZ.fX + mA[7] * srcXYZ.fY + mA[8] * srcXYZ.fZ;
+ SkVector3 dstCone;
+ dstCone.fX = mA[0] * dstXYZ.fX + mA[1] * dstXYZ.fY + mA[2] * dstXYZ.fZ;
+ dstCone.fY = mA[3] * dstXYZ.fX + mA[4] * dstXYZ.fY + mA[5] * dstXYZ.fZ;
+ dstCone.fZ = mA[6] * dstXYZ.fX + mA[7] * dstXYZ.fY + mA[8] * dstXYZ.fZ;
+
+ SkMatrix DXToD50;
+ DXToD50.setIdentity();
+ DXToD50[0] = dstCone.fX / srcCone.fX;
+ DXToD50[4] = dstCone.fY / srcCone.fY;
+ DXToD50[8] = dstCone.fZ / srcCone.fZ;
+ DXToD50.postConcat(mAInv);
+ DXToD50.preConcat(mA);
+
+ SkMatrix toXYZ3x3;
+ toXYZ3x3.setAll(toXYZ[0], toXYZ[3], toXYZ[6], toXYZ[1], toXYZ[4], toXYZ[7], toXYZ[2], toXYZ[5],
+ toXYZ[8]);
+ toXYZ3x3.postConcat(DXToD50);
+
+ toXYZD50->set3x3(toXYZ3x3[0], toXYZ3x3[1], toXYZ3x3[2], toXYZ3x3[3], toXYZ3x3[4], toXYZ3x3[5],
+ toXYZ3x3[6], toXYZ3x3[7], toXYZ3x3[8]);
+ return true;
+}
+
// Returns a colorSpace object that represents any color space information in
// the encoded data. If the encoded data contains no color space, this will
// return NULL.
@@ -213,24 +270,28 @@
}
// Next, check for chromaticities.
- png_fixed_point XYZ[9];
- float toXYZD50[9];
+ png_fixed_point toXYZFixed[9];
+ float toXYZ[9];
+ png_fixed_point whitePointFixed[2];
+ float whitePoint[2];
png_fixed_point gamma;
float gammas[3];
- if (png_get_cHRM_XYZ_fixed(png_ptr, info_ptr, &XYZ[0], &XYZ[1], &XYZ[2], &XYZ[3], &XYZ[4],
- &XYZ[5], &XYZ[6], &XYZ[7], &XYZ[8])) {
-
- // FIXME (msarett): Here we are treating XYZ values as D50 even though the color
- // temperature is unspecified. I suspect that this assumption
- // is most often ok, but we could also calculate the color
- // temperature (D value) and then convert the XYZ to D50. Maybe
- // we should add a new constructor to SkColorSpace that accepts
- // XYZ with D-Unkown?
+ if (png_get_cHRM_XYZ_fixed(png_ptr, info_ptr, &toXYZFixed[0], &toXYZFixed[1], &toXYZFixed[2],
+ &toXYZFixed[3], &toXYZFixed[4], &toXYZFixed[5], &toXYZFixed[6],
+ &toXYZFixed[7], &toXYZFixed[8]) &&
+ png_get_cHRM_fixed(png_ptr, info_ptr, &whitePointFixed[0], &whitePointFixed[1], nullptr,
+ nullptr, nullptr, nullptr, nullptr, nullptr))
+ {
for (int i = 0; i < 9; i++) {
- toXYZD50[i] = png_fixed_point_to_float(XYZ[i]);
+ toXYZ[i] = png_fixed_point_to_float(toXYZFixed[i]);
}
- SkMatrix44 mat(SkMatrix44::kUninitialized_Constructor);
- mat.set3x3RowMajorf(toXYZD50);
+ whitePoint[0] = png_fixed_point_to_float(whitePointFixed[0]);
+ whitePoint[1] = png_fixed_point_to_float(whitePointFixed[1]);
+
+ SkMatrix44 toXYZD50(SkMatrix44::kUninitialized_Constructor);
+ if (!convert_to_D50(&toXYZD50, toXYZ, whitePoint)) {
+ toXYZD50.set3x3RowMajorf(gSRGB_toXYZD50);
+ }
if (PNG_INFO_gAMA == png_get_gAMA_fixed(png_ptr, info_ptr, &gamma)) {
float value = png_inverted_fixed_point_to_float(gamma);
@@ -238,12 +299,12 @@
gammas[1] = value;
gammas[2] = value;
- return SkColorSpace_Base::NewRGB(gammas, mat);
+ return SkColorSpace_Base::NewRGB(gammas, toXYZD50);
}
// Default to sRGB gamma if the image has color space information,
// but does not specify gamma.
- return SkColorSpace::NewRGB(SkColorSpace::kSRGB_GammaNamed, mat);
+ return SkColorSpace::NewRGB(SkColorSpace::kSRGB_GammaNamed, toXYZD50);
}
// Last, check for gamma.
@@ -256,10 +317,10 @@
gammas[2] = value;
// Since there is no cHRM, we will guess sRGB gamut.
- SkMatrix44 mat(SkMatrix44::kUninitialized_Constructor);
- mat.set3x3RowMajorf(gSRGB_toXYZD50);
+ SkMatrix44 toXYZD50(SkMatrix44::kUninitialized_Constructor);
+ toXYZD50.set3x3RowMajorf(gSRGB_toXYZD50);
- return SkColorSpace_Base::NewRGB(gammas, mat);
+ return SkColorSpace_Base::NewRGB(gammas, toXYZD50);
}
#endif // LIBPNG >= 1.6