AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix swapped interpretation of c and e in SkColorSpace_ICC

Browse Source 
The ICC errata supports the opposite of what we do.
http://www.color.org/icc_specs2.xalter

TBR=reed@google.com

BUG=skia:

CQ_INCLUDE_TRYBOTS=skia.primary:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD

Change-Id: I18ace7f312926b264e624c30d8cb983eff5c434b
Reviewed-on: https://skia-review.googlesource.com/6277
Commit-Queue: Matt Sarett <msarett@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This commit is contained in:
Matt Sarett 2016-12-19 14:33:35 -05:00 committed by Skia Commit-Bot
parent 65869fb64b
commit 2410717f90
8 changed files with 69 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
include/core/SkColorSpace.h
View File

@ -33,8 +33,8 @@ struct SK_API SkColorSpacePrimaries {
* Contains the coefficients for a common transfer function equation, specified as
* a transformation from a curved space to linear.
*
* LinearVal = E*InputVal + F , for 0.0f <= InputVal < D
* LinearVal = (A*InputVal + B)^G + C, for D <= InputVal <= 1.0f
* LinearVal = C*InputVal + F , for 0.0f <= InputVal < D
* LinearVal = (A*InputVal + B)^G + E, for D <= InputVal <= 1.0f
*
* Function is undefined if InputVal is not in [ 0.0f, 1.0f ].
* Resulting LinearVals must be in [ 0.0f, 1.0f ].

28
src/core/SkColorSpacePriv.h
View File

@ -45,7 +45,7 @@ static inline bool is_valid_transfer_fn(const SkColorSpaceTransferFn& coeffs) {
}
}
if (coeffs.fD == 1.0f) {
if (coeffs.fD >= 1.0f) {
// Y = eX + f for always
if (0.0f == coeffs.fE) {
SkColorSpacePrintf("E is zero, constant transfer function is "
@ -54,13 +54,13 @@ static inline bool is_valid_transfer_fn(const SkColorSpaceTransferFn& coeffs) {
}
}
if ((0.0f == coeffs.fA || 0.0f == coeffs.fG) && 0.0f == coeffs.fE) {
if ((0.0f == coeffs.fA || 0.0f == coeffs.fG) && 0.0f == coeffs.fC) {
SkColorSpacePrintf("A or G, and E are zero, constant transfer function "
"is nonsense");
return false;
}
if (coeffs.fE < 0.0f) {
if (coeffs.fC < 0.0f) {
SkColorSpacePrintf("Transfer function must be increasing");
return false;
}
@ -74,13 +74,13 @@ static inline bool is_valid_transfer_fn(const SkColorSpaceTransferFn& coeffs) {
}
static inline bool is_almost_srgb(const SkColorSpaceTransferFn& coeffs) {
return color_space_almost_equal(0.9479f, coeffs.fA) &&
color_space_almost_equal(0.0521f, coeffs.fB) &&
color_space_almost_equal(0.0000f, coeffs.fC) &&
color_space_almost_equal(0.0405f, coeffs.fD) &&
color_space_almost_equal(0.0774f, coeffs.fE) &&
color_space_almost_equal(0.0000f, coeffs.fF) &&
color_space_almost_equal(2.4000f, coeffs.fG);
return color_space_almost_equal(1.0f / 1.055f, coeffs.fA) &&
color_space_almost_equal(0.055f / 1.055f, coeffs.fB) &&
color_space_almost_equal(1.0f / 12.92f, coeffs.fC) &&
color_space_almost_equal(0.04045f, coeffs.fD) &&
color_space_almost_equal(0.00000f, coeffs.fE) &&
color_space_almost_equal(0.00000f, coeffs.fF) &&
color_space_almost_equal(2.40000f, coeffs.fG);
}
static inline bool is_almost_2dot2(const SkColorSpaceTransferFn& coeffs) {
@ -109,9 +109,9 @@ static inline bool named_to_parametric(SkColorSpaceTransferFn* coeffs,
case kSRGB_SkGammaNamed:
coeffs->fA = 1.0f / 1.055f;
coeffs->fB = 0.055f / 1.055f;
coeffs->fC = 0.0f;
coeffs->fC = 1.0f / 12.92f;
coeffs->fD = 0.04045f;
coeffs->fE = 1.0f / 12.92f;
coeffs->fE = 0.0f;
coeffs->fF = 0.0f;
coeffs->fG = 2.4f;
return true;
@ -121,11 +121,11 @@ static inline bool named_to_parametric(SkColorSpaceTransferFn* coeffs,
case kLinear_SkGammaNamed:
coeffs->fA = 0.0f;
coeffs->fB = 0.0f;
coeffs->fC = 0.0f;
coeffs->fC = 1.0f;
// Make sure that we use the linear segment of the transfer function even
// when the x-value is 1.0f.
coeffs->fD = add_epsilon(1.0f);
coeffs->fE = 1.0f;
coeffs->fE = 0.0f;
coeffs->fF = 0.0f;
coeffs->fG = 0.0f;
return true;

24
src/core/SkColorSpaceXform.cpp
View File

@ -116,13 +116,13 @@ static void build_table_linear_from_gamma(float* outTable, const float* inTable,
static void build_table_linear_from_gamma(float* outTable, float g, float a, float b, float c,
float d, float e, float f) {
// Y = (aX + b)^g + c for X >= d
// Y = eX + f otherwise
// Y = (aX + b)^g + e for X >= d
// Y = cX + f otherwise
for (float x = 0.0f; x <= 1.0f; x += (1.0f/255.0f)) {
if (x >= d) {
*outTable++ = clamp_0_1(powf(a * x + b, g) + c);
*outTable++ = clamp_0_1(powf(a * x + b, g) + e);
} else {
*outTable++ = clamp_0_1(e * x + f);
*outTable++ = clamp_0_1(c * x + f);
}
}
}
@ -154,26 +154,26 @@ static float inverse_parametric(float x, float g, float a, float b, float c, flo
// Assume that the gamma function is continuous, or this won't make much sense anyway.
// Plug in |d| to the first equation to calculate the new piecewise interval.
// Then simply use the inverse of the original functions.
float interval = e * d + f;
float interval = c * d + f;
if (x < interval) {
// X = (Y - F) / E
if (0.0f == e) {
// X = (Y - F) / C
if (0.0f == c) {
// The gamma curve for this segment is constant, so the inverse is undefined.
// Since this is the lower segment, guess zero.
return 0.0f;
}
return (x - f) / e;
return (x - f) / c;
}
// X = ((Y - C)^(1 / G) - B) / A
// X = ((Y - E)^(1 / G) - B) / A
if (0.0f == a || 0.0f == g) {
// The gamma curve for this segment is constant, so the inverse is undefined.
// Since this is the upper segment, guess one.
return 1.0f;
}
return (powf(x - c, 1.0f / g) - b) / a;
return (powf(x - e, 1.0f / g) - b) / a;
}
static void build_table_linear_to_gamma(uint8_t* outTable, float g, float a,
@ -237,8 +237,8 @@ static void build_gamma_tables(const T* outGammaTables[3], T* gammaTableStorage,
switch (gammas->data(i).fNamed) {
case kSRGB_SkGammaNamed:
(*fns.fBuildFromParam)(&gammaTableStorage[i * gammaTableSize], 2.4f,
(1.0f / 1.055f), (0.055f / 1.055f), 0.0f,
0.04045f, (1.0f / 12.92f), 0.0f);
(1.0f / 1.055f), (0.055f / 1.055f),
(1.0f / 12.92f), 0.04045f, 0.0f, 0.0f);
outGammaTables[i] = &gammaTableStorage[i * gammaTableSize];
break;
case k2Dot2Curve_SkGammaNamed:

40
src/core/SkColorSpaceXform_A2B.cpp
View File

@ -85,49 +85,49 @@ static inline bool gamma_to_parametric(SkColorSpaceTransferFn* coeffs, const SkG
}
}
static inline SkColorSpaceTransferFn invert_parametric(const SkColorSpaceTransferFn& fn) {
// Original equation is: y = (ax + b)^g + c for x >= d
// y = ex + f otherwise
// Original equation is: y = (ax + b)^g + e for x >= d
// y = cx + f otherwise
//
// so 1st inverse is: (y - c)^(1/g) = ax + b
// x = ((y - c)^(1/g) - b) / a
// so 1st inverse is: (y - e)^(1/g) = ax + b
// x = ((y - e)^(1/g) - b) / a
//
// which can be re-written as: x = (1/a)(y - c)^(1/g) - b/a
// x = ((1/a)^g)^(1/g) * (y - c)^(1/g) - b/a
// x = ([(1/a)^g]y + [-((1/a)^g)c]) ^ [1/g] + [-b/a]
// which can be re-written as: x = (1/a)(y - e)^(1/g) - b/a
// x = ((1/a)^g)^(1/g) * (y - e)^(1/g) - b/a
// x = ([(1/a)^g]y + [-((1/a)^g)e]) ^ [1/g] + [-b/a]
//
// and 2nd inverse is: x = (y - f) / e
// which can be re-written as: x = [1/e]y + [-f/e]
// and 2nd inverse is: x = (y - f) / c
// which can be re-written as: x = [1/c]y + [-f/c]
//
// and now both can be expressed in terms of the same parametric form as the
// original - parameters are enclosed in square brackets.
// find inverse for linear segment (if possible)
float e, f;
if (0.f == fn.fE) {
float c, f;
if (0.f == fn.fC) {
// otherwise assume it should be 0 as it is the lower segment
// as y = f is a constant function
e = 0.f;
c = 0.f;
f = 0.f;
} else {
e = 1.f / fn.fE;
f = -fn.fF / fn.fE;
c = 1.f / fn.fC;
f = -fn.fF / fn.fC;
}
// find inverse for the other segment (if possible)
float g, a, b, c;
float g, a, b, e;
if (0.f == fn.fA || 0.f == fn.fG) {
// otherwise assume it should be 1 as it is the top segment
// as you can't invert the constant functions y = b^g + c, or y = 1 + c
g = 1.f;
a = 0.f;
b = 0.f;
c = 1.f;
e = 1.f;
} else {
g = 1.f / fn.fG;
a = powf(1.f / fn.fA, fn.fG);
b = -a * fn.fC;
c = -fn.fB / fn.fA;
b = -a * fn.fE;
e = -fn.fB / fn.fA;
}
const float d = fn.fE * fn.fD + fn.fF;
const float d = fn.fC * fn.fD + fn.fF;
return {g, a, b, c, d, e, f};
}
@ -152,7 +152,7 @@ SkColorSpaceXform_A2B::SkColorSpaceXform_A2B(SkColorSpace_A2B* srcSpace,
// CMYK images from JPEGs (the only format that supports it) are actually
// inverted CMYK, so we need to invert every channel.
// TransferFn is y = -x + 1 for x < 1.f, otherwise 0x + 0, ie y = 1 - x for x in [0,1]
this->addTransferFns({1.f, 0.f, 0.f, 0.f, 1.f, -1.f, 1.f}, 4);
this->addTransferFns({1.f, 0.f, 0.f, -1.f, 1.f, 0.f, 1.f}, 4);
break;
case SkColorSpace_Base::InputColorFormat::kGray:
currentChannels = 1;

27
src/core/SkColorSpace_ICC.cpp
View File

@ -407,8 +407,8 @@ static SkGammas::Type parse_gamma(SkGammas::Data* outData, SkColorSpaceTransferF
// Here's where the real parametric gammas start. There are many
// permutations of the same equations.
//
// Y = (aX + b)^g + c for X >= d
// Y = eX + f otherwise
// Y = (aX + b)^g + e for X >= d
// Y = cX + f otherwise
//
// We will fill in with zeros as necessary to always match the above form.
if (len < 24) {
@ -434,11 +434,11 @@ static SkGammas::Type parse_gamma(SkGammas::Data* outData, SkColorSpaceTransferF
return SkGammas::Type::kNone_Type;
}
// Y = (aX + b)^g + c for X >= -b/a
// Y = c otherwise
c = read_big_endian_16_dot_16(src + 24);
// Y = (aX + b)^g + e for X >= -b/a
// Y = e otherwise
e = read_big_endian_16_dot_16(src + 24);
d = -b / a;
f = c;
f = e;
break;
case kGABDE_ParaCurveType:
tagBytes = 12 + 20;
@ -448,14 +448,9 @@ static SkGammas::Type parse_gamma(SkGammas::Data* outData, SkColorSpaceTransferF
}
// Y = (aX + b)^g for X >= d
// Y = eX otherwise
// Y = cX otherwise
c = read_big_endian_16_dot_16(src + 24);
d = read_big_endian_16_dot_16(src + 28);
// Not a bug! We define |e| to always be the coefficient on X in the
// second equation. The spec calls this |c| in this particular equation.
// We don't follow their convention because then |c| would have a
// different meaning in each of our cases.
e = read_big_endian_16_dot_16(src + 24);
break;
case kGABCDEF_ParaCurveType:
tagBytes = 12 + 28;
@ -464,10 +459,8 @@ static SkGammas::Type parse_gamma(SkGammas::Data* outData, SkColorSpaceTransferF
return SkGammas::Type::kNone_Type;
}
// Y = (aX + b)^g + c for X >= d
// Y = eX + f otherwise
// NOTE: The ICC spec writes "cX" in place of "eX" but I think
// it's a typo.
// Y = (aX + b)^g + e for X >= d
// Y = cX + f otherwise
c = read_big_endian_16_dot_16(src + 24);
d = read_big_endian_16_dot_16(src + 28);
e = read_big_endian_16_dot_16(src + 32);

4
src/opts/SkRasterPipeline_opts.h
View File

@ -685,8 +685,8 @@ SI SkNf parametric(const SkNf& v, const SkColorSpaceTransferFn& p) {
float result[N]; // Unconstrained powf() doesn't vectorize well...
for (int i = 0; i < N; i++) {
float s = v[i];
result[i] = (s <= p.fD) ? p.fE * s + p.fF
: powf(s * p.fA + p.fB, p.fG) + p.fC;
result[i] = (s <= p.fD) ? p.fC * s + p.fF
: powf(s * p.fA + p.fB, p.fG) + p.fE;
}
// Clamp the output to [0, 1].
// Max(NaN, 0) = 0, but Max(0, NaN) = NaN, so we want this exact order to ensure NaN => 0

8
tests/ColorSpaceTest.cpp
View File

@ -240,9 +240,9 @@ DEF_TEST(ColorSpace_Serialize, r) {
SkColorSpaceTransferFn fn;
fn.fA = 1.0f;
fn.fB = 0.0f;
fn.fC = 0.0f;
fn.fC = 1.0f;
fn.fD = 0.5f;
fn.fE = 1.0f;
fn.fE = 0.0f;
fn.fF = 0.0f;
fn.fG = 1.0f;
SkMatrix44 toXYZ(SkMatrix44::kIdentity_Constructor);
@ -265,9 +265,9 @@ DEF_TEST(ColorSpace_Equals, r) {
SkColorSpaceTransferFn fn;
fn.fA = 1.0f;
fn.fB = 0.0f;
fn.fC = 0.0f;
fn.fC = 1.0f;
fn.fD = 0.5f;
fn.fE = 1.0f;
fn.fE = 0.0f;
fn.fF = 0.0f;
fn.fG = 1.0f;
SkMatrix44 toXYZ(SkMatrix44::kIdentity_Constructor);

10
tests/ColorSpaceXformTest.cpp
View File

@ -176,18 +176,18 @@ DEF_TEST(ColorSpaceXform_ParametricGamma, r) {
SkColorSpaceTransferFn* params = SkTAddOffset<SkColorSpaceTransferFn>
(memory, sizeof(SkGammas));
// Interval, switch xforms at 0.0031308f
// Interval.
params->fD = 0.04045f;
// First equation:
params->fE = 1.0f / 12.92f;
params->fC = 1.0f / 12.92f;
params->fF = 0.0f;
// Second equation:
// Note that the function is continuous (it's actually sRGB).
params->fA = 1.0f / 1.055f;
params->fB = 0.055f / 1.055f;
params->fC = 0.0f;
params->fE = 0.0f;
params->fG = 2.4f;
test_identity_xform(r, gammas, true);
test_identity_xform_A2B(r, kNonStandard_SkGammaNamed, gammas);
@ -239,9 +239,9 @@ DEF_TEST(ColorSpaceXform_NonMatchingGamma, r) {
sizeof(SkGammas) + sizeof(float) * tableSize);
params->fA = 1.0f / 1.055f;
params->fB = 0.055f / 1.055f;
params->fC = 0.0f;
params->fC = 1.0f / 12.92f;
params->fD = 0.04045f;
params->fE = 1.0f / 12.92f;
params->fE = 0.0f;
params->fF = 0.0f;
params->fG = 2.4f;