diff options
author | 2016-12-19 14:33:35 -0500 | |
---|---|---|
committer | 2016-12-19 20:23:03 +0000 | |
commit | 2410717f900c2691db880d84a2e03a6a24905ee2 (patch) | |
tree | c89ffb3c18cc289fc68f37e166235dc36456b74b /src/core | |
parent | 65869fb64b56a4c59d74003c1fac5dffc8a8bf65 (diff) |
Fix swapped interpretation of c and e in SkColorSpace_ICC
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>
Diffstat (limited to 'src/core')
-rw-r--r-- | src/core/SkColorSpacePriv.h | 28 | ||||
-rw-r--r-- | src/core/SkColorSpaceXform.cpp | 24 | ||||
-rw-r--r-- | src/core/SkColorSpaceXform_A2B.cpp | 40 | ||||
-rw-r--r-- | src/core/SkColorSpace_ICC.cpp | 27 |
4 files changed, 56 insertions, 63 deletions
diff --git a/src/core/SkColorSpacePriv.h b/src/core/SkColorSpacePriv.h index 32981ae875..f1ef8dc6d6 100644 --- a/src/core/SkColorSpacePriv.h +++ b/src/core/SkColorSpacePriv.h @@ -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; diff --git a/src/core/SkColorSpaceXform.cpp b/src/core/SkColorSpaceXform.cpp index fb603db45e..6eaab832f0 100644 --- a/src/core/SkColorSpaceXform.cpp +++ b/src/core/SkColorSpaceXform.cpp @@ -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: diff --git a/src/core/SkColorSpaceXform_A2B.cpp b/src/core/SkColorSpaceXform_A2B.cpp index cb8dd57f9d..f65df4c409 100644 --- a/src/core/SkColorSpaceXform_A2B.cpp +++ b/src/core/SkColorSpaceXform_A2B.cpp @@ -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; diff --git a/src/core/SkColorSpace_ICC.cpp b/src/core/SkColorSpace_ICC.cpp index b76a981743..adb54565d6 100644 --- a/src/core/SkColorSpace_ICC.cpp +++ b/src/core/SkColorSpace_ICC.cpp @@ -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); |