From 113d05fa7b26797e3e468f78ea94a214476b63fb Mon Sep 17 00:00:00 2001 From: Ravi Mistry Date: Sat, 17 Dec 2016 01:31:03 +0000 Subject: Revert "Revert "WIP: Skia support library for ICC tasks"" This reverts commit eb733fbf56538838a36814c75cd03f917462cb22. Reason for revert: Revert patch was automatically merged incorrectly? Original change's description: > Revert "WIP: Skia support library for ICC tasks" > > This reverts commit fc8dc3194acb959ee5980b41766660ca0644bcab. > > Reason for revert: Breaks Build-Mac-Clang-Arm7-{Debug,Release}-iOS builds. > Example tasks: > * https://chromium-swarm.appspot.com/task?id=3322f668620b9e10&refresh=10 > * https://chromium-swarm.appspot.com/task?id=332296146331e810&refresh=10 > > Original change's description: > > WIP: Skia support library for ICC tasks > > > > As a starting point, this would be mostly trivial to implement using > > SkColorSpace. > > > > This also would give us the flexibility to begin to move all of > > the ICC related code from SkColorSpace to SkICC. > > > > What are the advantages of moving this away from SkColorSpace? > > (1) A long term goal (once Chrome uses SkCodec), might be to > > move SkColorSpace::MakeICC() out of the public API. That way, > > we can guarantee that we can draw to/from *any* SkColorSpace. > > (2) Keeps SkColorSpace separate from ICC-specific representations > > like SkColorSpaceTransferFn etc. > > > > BUG=skia: > > > > Change-Id: Iddeb9903221fb57fbfc01218d8641c928b4a5165 > > Reviewed-on: https://skia-review.googlesource.com/5676 > > Commit-Queue: Matt Sarett > > Reviewed-by: Brian Osman > > Reviewed-by: Mike Reed > > > > TBR=mtklein@google.com,msarett@google.com,brianosman@google.com,reed@google.com,reviews@skia.org > BUG=skia: > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > > Change-Id: Ibdf272fce25892402bd3e85595fb8814cdf59856 > Reviewed-on: https://skia-review.googlesource.com/6232 > Commit-Queue: Ravi Mistry > Reviewed-by: Ravi Mistry > TBR=mtklein@google.com,rmistry@google.com,msarett@google.com,reviews@skia.org,brianosman@google.com,reed@google.com BUG=skia: NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Change-Id: I68b1624cfab8adfe31b17e1193a7766507dec8b0 Reviewed-on: https://skia-review.googlesource.com/6233 Commit-Queue: Ravi Mistry Reviewed-by: Ravi Mistry --- gn/core.gni | 1 + gn/tests.gni | 1 + include/core/SkICC.h | 73 ++++++++++++++++++++++++++++ src/core/SkColorSpacePriv.h | 51 +++++++++++++++++++- src/core/SkColorSpaceXform_A2B.cpp | 56 +++++++++------------- src/core/SkColorSpace_A2B.h | 13 ++--- src/core/SkColorSpace_Base.h | 12 +++-- src/core/SkColorSpace_XYZ.cpp | 26 +++++++++- src/core/SkColorSpace_XYZ.h | 2 + src/core/SkICC.cpp | 36 ++++++++++++++ tests/ICCTest.cpp | 97 ++++++++++++++++++++++++++++++++++++++ 11 files changed, 320 insertions(+), 48 deletions(-) create mode 100644 include/core/SkICC.h create mode 100644 src/core/SkICC.cpp create mode 100644 tests/ICCTest.cpp diff --git a/gn/core.gni b/gn/core.gni index d88ac5a299..2e457531c8 100644 --- a/gn/core.gni +++ b/gn/core.gni @@ -153,6 +153,7 @@ skia_core_sources = [ "$_src/core/SkGraphics.cpp", "$_src/core/SkHalf.cpp", "$_src/core/SkHalf.h", + "$_src/core/SkICC.cpp", "$_src/core/SkImageFilter.cpp", "$_src/core/SkImageFilterCache.cpp", "$_src/core/SkImageFilterCache.h", diff --git a/gn/tests.gni b/gn/tests.gni index 5f2d8b7931..79fd4be34c 100644 --- a/gn/tests.gni +++ b/gn/tests.gni @@ -99,6 +99,7 @@ tests_sources = [ "$_tests/GrTRecorderTest.cpp", "$_tests/HashTest.cpp", "$_tests/image-bitmap.cpp", + "$_tests/ICCTest.cpp", "$_tests/ImageCacheTest.cpp", "$_tests/ImageFilterCacheTest.cpp", "$_tests/ImageFilterTest.cpp", diff --git a/include/core/SkICC.h b/include/core/SkICC.h new file mode 100644 index 0000000000..3780498df9 --- /dev/null +++ b/include/core/SkICC.h @@ -0,0 +1,73 @@ +/* + * Copyright 2016 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#ifndef SkICC_DEFINED +#define SkICC_DEFINED + +#include "SkRefCnt.h" + +struct SkColorSpaceTransferFn; +class SkColorSpace; +class SkData; +class SkMatrix44; + +class SK_API SkICC : public SkRefCnt { +public: + + /** + * Parse an ICC profile. + * + * Returns nullptr if the data is not a valid ICC profile or if the profile + * input space is not RGB. + */ + static sk_sp Make(const void*, size_t); + + /** + * If the gamut can be represented as transformation into XYZ D50, returns + * true and sets the proper values in |toXYZD50|. + * + * If not, returns false. This indicates that the ICC data is too complex + * to isolate a simple gamut transformation. + */ + bool toXYZD50(SkMatrix44* toXYZD50) const; + + /** + * If the transfer function can be represented as coefficients to the standard + * equation, returns true and sets |fn| to the proper values. + * + * If not, returns false. This indicates one of the following: + * (1) The R, G, and B transfer functions are not the same. + * (2) The transfer function is represented as a table that we have not managed + * to match to a standard curve. + * (3) The ICC data is too complex to isolate a single transfer function. + */ + bool isNumericalTransferFn(SkColorSpaceTransferFn* fn) const; + + /** + * If the transfer function can be approximated as coefficients to the standard + * equation, returns true and sets |fn| to the proper values. + * + * If not, returns false. This indicates one of the following: + * (1) The R, G, and B transfer functions are not the same. + * (2) The transfer function is represented as a table that is not increasing with + * end points at zero and one. + * (3) The ICC data is too complex to isolate a single transfer function. + */ + bool approximateNumericalTransferFn(SkColorSpaceTransferFn* fn) const; + + /** + * Write an ICC profile with transfer function |fn| and gamut |toXYZD50|. + */ + static sk_sp WriteToICC(const SkColorSpaceTransferFn& fn, const SkMatrix44& toXYZD50); + +private: + SkICC(sk_sp colorSpace); + + sk_sp fColorSpace; +}; + +#endif diff --git a/src/core/SkColorSpacePriv.h b/src/core/SkColorSpacePriv.h index 73038738ff..32981ae875 100644 --- a/src/core/SkColorSpacePriv.h +++ b/src/core/SkColorSpacePriv.h @@ -5,14 +5,22 @@ * found in the LICENSE file. */ +#include + #define SkColorSpacePrintf(...) static inline bool color_space_almost_equal(float a, float b) { return SkTAbs(a - b) < 0.01f; } +static inline float add_epsilon(float v) { + return v + FLT_MIN; +} + static inline bool is_zero_to_one(float v) { - return (0.0f <= v) && (v <= 1.0f); + // Because we allow a value just barely larger than 1, the client can use an + // entirely linear transfer function. + return (0.0f <= v) && (v <= add_epsilon(1.0f)); } static inline bool is_valid_transfer_fn(const SkColorSpaceTransferFn& coeffs) { @@ -84,3 +92,44 @@ static inline bool is_almost_2dot2(const SkColorSpaceTransferFn& coeffs) { color_space_almost_equal(0.0f, coeffs.fF) && color_space_almost_equal(2.2f, coeffs.fG); } + +static inline void value_to_parametric(SkColorSpaceTransferFn* coeffs, float exponent) { + coeffs->fA = 1.0f; + coeffs->fB = 0.0f; + coeffs->fC = 0.0f; + coeffs->fD = 0.0f; + coeffs->fE = 0.0f; + coeffs->fF = 0.0f; + coeffs->fG = exponent; +} + +static inline bool named_to_parametric(SkColorSpaceTransferFn* coeffs, + SkGammaNamed gammaNamed) { + switch (gammaNamed) { + case kSRGB_SkGammaNamed: + coeffs->fA = 1.0f / 1.055f; + coeffs->fB = 0.055f / 1.055f; + coeffs->fC = 0.0f; + coeffs->fD = 0.04045f; + coeffs->fE = 1.0f / 12.92f; + coeffs->fF = 0.0f; + coeffs->fG = 2.4f; + return true; + case k2Dot2Curve_SkGammaNamed: + value_to_parametric(coeffs, 2.2f); + return true; + case kLinear_SkGammaNamed: + coeffs->fA = 0.0f; + coeffs->fB = 0.0f; + coeffs->fC = 0.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->fF = 0.0f; + coeffs->fG = 0.0f; + return true; + default: + return false; + } +} diff --git a/src/core/SkColorSpaceXform_A2B.cpp b/src/core/SkColorSpaceXform_A2B.cpp index 83f1d910ae..cb8dd57f9d 100644 --- a/src/core/SkColorSpaceXform_A2B.cpp +++ b/src/core/SkColorSpaceXform_A2B.cpp @@ -69,35 +69,19 @@ bool SkColorSpaceXform_A2B::onApply(ColorFormat dstFormat, void* dst, ColorForma return true; } -static inline SkColorSpaceTransferFn value_to_parametric(float exp) { - return {exp, 1.f, 0.f, 0.f, 0.f, 0.f, 0.f}; -} - -static inline SkColorSpaceTransferFn gammanamed_to_parametric(SkGammaNamed gammaNamed) { - switch (gammaNamed) { - case kLinear_SkGammaNamed: - return value_to_parametric(1.f); - case kSRGB_SkGammaNamed: - return {2.4f, (1.f / 1.055f), (0.055f / 1.055f), 0.f, 0.04045f, (1.f / 12.92f), 0.f}; - case k2Dot2Curve_SkGammaNamed: - return value_to_parametric(2.2f); - default: - SkASSERT(false); - return {-1.f, -1.f, -1.f, -1.f, -1.f, -1.f, -1.f}; - } -} - -static inline SkColorSpaceTransferFn gamma_to_parametric(const SkGammas& gammas, int channel) { +static inline bool gamma_to_parametric(SkColorSpaceTransferFn* coeffs, const SkGammas& gammas, + int channel) { switch (gammas.type(channel)) { case SkGammas::Type::kNamed_Type: - return gammanamed_to_parametric(gammas.data(channel).fNamed); + return named_to_parametric(coeffs, gammas.data(channel).fNamed); case SkGammas::Type::kValue_Type: - return value_to_parametric(gammas.data(channel).fValue); + value_to_parametric(coeffs, gammas.data(channel).fValue); + return true; case SkGammas::Type::kParam_Type: - return gammas.params(channel); + *coeffs = gammas.params(channel); + return true; default: - SkASSERT(false); - return {-1.f, -1.f, -1.f, -1.f, -1.f, -1.f, -1.f}; + return false; } } static inline SkColorSpaceTransferFn invert_parametric(const SkColorSpaceTransferFn& fn) { @@ -183,6 +167,10 @@ SkColorSpaceXform_A2B::SkColorSpaceXform_A2B(SkColorSpace_A2B* srcSpace, currentChannels = e.outputChannels(); switch (e.type()) { case SkColorSpace_A2B::Element::Type::kGammaNamed: + if (kLinear_SkGammaNamed == e.gammaNamed()) { + break; + } + // take the fast path for 3-channel named gammas if (3 == currentChannels) { if (k2Dot2Curve_SkGammaNamed == e.gammaNamed()) { @@ -196,12 +184,11 @@ SkColorSpaceXform_A2B::SkColorSpaceXform_A2B(SkColorSpace_A2B* srcSpace, break; } } - if (kLinear_SkGammaNamed != e.gammaNamed()) { - SkCSXformPrintf("Gamma stage added: %s\n", - debugGammaNamed[(int)e.gammaNamed()]); - SkColorSpaceTransferFn fn = gammanamed_to_parametric(e.gammaNamed()); - this->addTransferFns(fn, currentChannels); - } + + SkCSXformPrintf("Gamma stage added: %s\n", debugGammaNamed[(int)e.gammaNamed()]); + SkColorSpaceTransferFn fn; + SkAssertResult(named_to_parametric(&fn, e.gammaNamed())); + this->addTransferFns(fn, currentChannels); break; case SkColorSpace_A2B::Element::Type::kGammas: { const SkGammas& gammas = e.gammas(); @@ -221,7 +208,8 @@ SkColorSpaceXform_A2B::SkColorSpaceXform_A2B(SkColorSpace_A2B* srcSpace, this->addTableFn(table, channel); gammaNeedsRef = true; } else { - SkColorSpaceTransferFn fn = gamma_to_parametric(gammas, channel); + SkColorSpaceTransferFn fn; + SkAssertResult(gamma_to_parametric(&fn, gammas, channel)); this->addTransferFn(fn, channel); } } @@ -301,9 +289,9 @@ SkColorSpaceXform_A2B::SkColorSpaceXform_A2B(SkColorSpace_A2B* srcSpace, this->addTableFn(table, channel); } else { - SkColorSpaceTransferFn fn = - invert_parametric(gamma_to_parametric(gammas, channel)); - this->addTransferFn(fn, channel); + SkColorSpaceTransferFn fn; + SkAssertResult(gamma_to_parametric(&fn, gammas, channel)); + this->addTransferFn(invert_parametric(fn), channel); } } } diff --git a/src/core/SkColorSpace_A2B.h b/src/core/SkColorSpace_A2B.h index ed90713a2c..084c5934ce 100644 --- a/src/core/SkColorSpace_A2B.h +++ b/src/core/SkColorSpace_A2B.h @@ -47,15 +47,10 @@ public: return nullptr; } - bool onGammaCloseToSRGB() const override { - // There is no single gamma curve in an A2B0 profile - return false; - } - - bool onGammaIsLinear() const override { - // There is no single gamma curve in an A2B0 profile - return false; - } + // There is no single gamma curve in an A2B0 profile + bool onGammaCloseToSRGB() const override { return false; } + bool onGammaIsLinear() const override { return false; } + bool onIsNumericalTransferFn(SkColorSpaceTransferFn* coeffs) const override { return false; } sk_sp makeLinearGamma() override { // TODO: Analyze the extrema of our projection into XYZ and use suitable primaries? diff --git a/src/core/SkColorSpace_Base.h b/src/core/SkColorSpace_Base.h index e7dbf128b4..af8ec3133f 100644 --- a/src/core/SkColorSpace_Base.h +++ b/src/core/SkColorSpace_Base.h @@ -55,6 +55,10 @@ struct SkGammas : SkRefCnt { this->fTable.fSize == that.fTable.fSize; } + inline bool operator!=(const Data& that) const { + return !(*this == that); + } + SkGammaNamed fNamed; float fValue; Table fTable; @@ -153,11 +157,13 @@ public: * Returns nullptr if color gamut cannot be described in terms of XYZ D50. */ virtual const SkMatrix44* fromXYZD50() const = 0; - + virtual bool onGammaCloseToSRGB() const = 0; - + virtual bool onGammaIsLinear() const = 0; - + + virtual bool onIsNumericalTransferFn(SkColorSpaceTransferFn* coeffs) const = 0; + /** * Returns a color space with the same gamut as this one, but with a linear gamma. * For color spaces whose gamut can not be described in terms of XYZ D50, returns diff --git a/src/core/SkColorSpace_XYZ.cpp b/src/core/SkColorSpace_XYZ.cpp index b91789614f..30f70b6e26 100644 --- a/src/core/SkColorSpace_XYZ.cpp +++ b/src/core/SkColorSpace_XYZ.cpp @@ -5,8 +5,9 @@ * found in the LICENSE file. */ -#include "SkColorSpace_XYZ.h" #include "SkChecksum.h" +#include "SkColorSpace_XYZ.h" +#include "SkColorSpacePriv.h" #include "SkColorSpaceXform_Base.h" static constexpr float gSRGB_toXYZD50[] { @@ -64,6 +65,29 @@ bool SkColorSpace_XYZ::onGammaIsLinear() const { return kLinear_SkGammaNamed == fGammaNamed; } +bool SkColorSpace_XYZ::onIsNumericalTransferFn(SkColorSpaceTransferFn* coeffs) const { + if (named_to_parametric(coeffs, fGammaNamed)) { + return true; + } + + SkASSERT(fGammas); + if (fGammas->data(0) != fGammas->data(1) || fGammas->data(0) != fGammas->data(2)) { + return false; + } + + if (fGammas->isValue(0)) { + value_to_parametric(coeffs, fGammas->data(0).fValue); + return true; + } + + if (fGammas->isParametric(0)) { + *coeffs = fGammas->params(0); + return true; + } + + return false; +} + sk_sp SkColorSpace_XYZ::makeLinearGamma() { if (this->gammaIsLinear()) { return sk_ref_sp(this); diff --git a/src/core/SkColorSpace_XYZ.h b/src/core/SkColorSpace_XYZ.h index 4a32188d53..d0ce0f813e 100644 --- a/src/core/SkColorSpace_XYZ.h +++ b/src/core/SkColorSpace_XYZ.h @@ -23,6 +23,8 @@ public: bool onGammaIsLinear() const override; + bool onIsNumericalTransferFn(SkColorSpaceTransferFn* coeffs) const override; + Type type() const override { return Type::kXYZ; } sk_sp makeLinearGamma() override; diff --git a/src/core/SkICC.cpp b/src/core/SkICC.cpp new file mode 100644 index 0000000000..9a1a3c6e33 --- /dev/null +++ b/src/core/SkICC.cpp @@ -0,0 +1,36 @@ +/* + * Copyright 2016 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "SkColorSpace_Base.h" +#include "SkICC.h" + +SkICC::SkICC(sk_sp colorSpace) + : fColorSpace(std::move(colorSpace)) +{} + +sk_sp SkICC::Make(const void* ptr, size_t len) { + sk_sp colorSpace = SkColorSpace::MakeICC(ptr, len); + if (!colorSpace) { + return nullptr; + } + + return sk_sp(new SkICC(std::move(colorSpace))); +} + +bool SkICC::toXYZD50(SkMatrix44* toXYZD50) const { + const SkMatrix44* m = as_CSB(fColorSpace)->toXYZD50(); + if (!m) { + return false; + } + + *toXYZD50 = *m; + return true; +} + +bool SkICC::isNumericalTransferFn(SkColorSpaceTransferFn* coeffs) const { + return as_CSB(fColorSpace)->onIsNumericalTransferFn(coeffs); +} diff --git a/tests/ICCTest.cpp b/tests/ICCTest.cpp new file mode 100644 index 0000000000..f4639f0003 --- /dev/null +++ b/tests/ICCTest.cpp @@ -0,0 +1,97 @@ +/* + * Copyright 2016 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "Resources.h" +#include "SkColorSpace.h" +#include "SkData.h" +#include "SkICC.h" +#include "SkMatrix44.h" +#include "Test.h" + +static bool almost_equal(float a, float b) { + return SkTAbs(a - b) < 0.001f; +} + +static inline void test_to_xyz_d50(skiatest::Reporter* r, SkICC* icc, bool shouldSucceed, + const float* reference) { + SkMatrix44 result; + REPORTER_ASSERT(r, shouldSucceed == icc->toXYZD50(&result)); + if (shouldSucceed) { + float resultVals[16]; + result.asColMajorf(resultVals); + for (int i = 0; i < 16; i++) { + REPORTER_ASSERT(r, almost_equal(resultVals[i], reference[i])); + } + } +} + +DEF_TEST(ICC_ToXYZD50, r) { + const float z30Reference[16] = { + 0.59825f, 0.27103f, 0.00603f, 0.0f, 0.22243f, 0.67447f, 0.07368f, 0.0f, 0.14352f, 0.05449f, + 0.74519f, 0.0f, 0.0f, 0.0f, 0.0f, 1.0f, + }; + + sk_sp data = SkData::MakeFromFileName( + GetResourcePath("icc_profiles/HP_ZR30w.icc").c_str()); + sk_sp z30 = SkICC::Make(data->data(), data->size()); + test_to_xyz_d50(r, z30.get(), true, z30Reference); + + const float z32Reference[16] = { + 0.61583f, 0.28789f, 0.00513f, 0.0f, 0.20428f, 0.66972f, 0.06609f, 0.0f, 0.14409f, 0.04237f, + 0.75368f, 0.0f, 0.0f, 0.0f, 0.0f, 1.0f, + }; + + data = SkData::MakeFromFileName( GetResourcePath("icc_profiles/HP_Z32x.icc").c_str()); + sk_sp z32 = SkICC::Make(data->data(), data->size()); + test_to_xyz_d50(r, z32.get(), true, z32Reference); + + data = SkData::MakeFromFileName(GetResourcePath("icc_profiles/upperLeft.icc").c_str()); + sk_sp upperLeft = SkICC::Make(data->data(), data->size()); + test_to_xyz_d50(r, upperLeft.get(), false, z32Reference); + + data = SkData::MakeFromFileName(GetResourcePath("icc_profiles/upperRight.icc").c_str()); + sk_sp upperRight = SkICC::Make(data->data(), data->size()); + test_to_xyz_d50(r, upperRight.get(), false, z32Reference); +} + +static inline void test_is_numerical_transfer_fn(skiatest::Reporter* r, SkICC* icc, + bool shouldSucceed, + const SkColorSpaceTransferFn& reference) { + SkColorSpaceTransferFn result; + REPORTER_ASSERT(r, shouldSucceed == icc->isNumericalTransferFn(&result)); + if (shouldSucceed) { + REPORTER_ASSERT(r, 0 == memcmp(&result, &reference, sizeof(SkColorSpaceTransferFn))); + } +} + +DEF_TEST(ICC_IsNumericalTransferFn, r) { + SkColorSpaceTransferFn referenceFn; + referenceFn.fA = 1.0f; + referenceFn.fB = 0.0f; + referenceFn.fC = 0.0f; + referenceFn.fD = 0.0f; + referenceFn.fE = 0.0f; + referenceFn.fF = 0.0f; + referenceFn.fG = 2.2f; + + sk_sp data = SkData::MakeFromFileName( + GetResourcePath("icc_profiles/HP_ZR30w.icc").c_str()); + sk_sp z30 = SkICC::Make(data->data(), data->size()); + test_is_numerical_transfer_fn(r, z30.get(), true, referenceFn); + + data = SkData::MakeFromFileName( GetResourcePath("icc_profiles/HP_Z32x.icc").c_str()); + sk_sp z32 = SkICC::Make(data->data(), data->size()); + test_is_numerical_transfer_fn(r, z32.get(), true, referenceFn); + + data = SkData::MakeFromFileName(GetResourcePath("icc_profiles/upperLeft.icc").c_str()); + sk_sp upperLeft = SkICC::Make(data->data(), data->size()); + test_is_numerical_transfer_fn(r, upperLeft.get(), false, referenceFn); + + data = SkData::MakeFromFileName(GetResourcePath("icc_profiles/upperRight.icc").c_str()); + sk_sp upperRight = SkICC::Make(data->data(), data->size()); + test_is_numerical_transfer_fn(r, upperRight.get(), false, referenceFn); +} -- cgit v1.2.3